review feat: finish the changing behaviour for noclasspath mode #2074
Conversation
surli
changed the title
surli
changed the title
| @@ -544,6 +544,7 @@ public void testUsageOfTypeAnnotationBeforeExceptionInSignatureOfMethod() throws | |||
| @Test | |||
| public void testUsageOfTypeAnnotationInReturnTypeInMethod() throws Exception { | |||
| final Launcher launcher = new Launcher(); | |||
| launcher.getEnvironment().setNoClasspath(false); | |||
There was a problem hiding this comment.
why do we need those changes? if it works in full classpath, it should work in noclasspath, correct?
There was a problem hiding this comment.
Actually it works but the printed result is not exactly the same, so instead of fixing the assert I decided to keep the old setup.
Here the obtained diff if we call the test with a noclasspath:
Return type with an type annotation must be well printed expected:
<[public [email protected]
]String m3() {
re...> but was:<[@spoon.test.annotation.testclasses.TypeAnnotation
public java.lang.]String m3() {
re...>
There was a problem hiding this comment.
does this mean that we don't have the fully-qualified names in noclasspath or not?
There was a problem hiding this comment.
No it means that in noclasspath, the annotation seems not printed exactly at the same place: before or after the modifier of a field.
src/main/java/spoon/Launcher.java
Outdated
| Launcher.LOGGER.warn("Spoon is now using the 'no classpath mode' by default. If you want to ensure using Spoon in full classpath mode, please use the new flag: --fullclasspath."); | ||
| } | ||
|
|
||
| if (jsapActualArgs.getBoolean("fullclasspath")) { |
There was a problem hiding this comment.
I don't understand why we need a new option which is also boolean.
We can still write --noclasspath true or --noclasspath false? We only have to change the default value?
There was a problem hiding this comment.
The argument is a switch, you cannot write --noclasspath true or false:
[urli@vivat ~]$ java -jar spoon-core-6.2.0-jar-with-dependencies.jar --noclasspath true
Error: Unexpected argument: true
So in fact you could only switch on the noclasspath, and never switch off it. The opposite option allows that.
There was a problem hiding this comment.
I see. Then instead of introducing a closed swith, I would suggest to introduce an open "--mode", "--mode noclasspath", "--mode fullclasspath", that we could extend in the future with new options.
There was a problem hiding this comment.
Would this mode be related only to classpath? If yes I would prefer to name it cpmode to avoid ambiguity.
And do we keep the old switch for backward compatibility or not?
There was a problem hiding this comment.
By the way, just for information you merge in #2065 a new switch in the usage to disable the comments: it was exactly the same case as this one.
|
I know :-) I hesitated before merging it ;-) But decided to go for it because we have plenty of
other things to do to finalize the release.
|
|
OK for cpmode.
Regarding keeping the old switch, what's the behavior of JSAP with unknown arguments: crash?
silence? warning?
|
|
Why?
|
It crashes. |
surli
changed the title
surli
changed the title
We change the behaviour of Spoon regarding the noclasspath mode in #1936 however we don't make any change about that on the command-line interface as pointed out in #2073.
This PR is a first proposal to align the behaviours.
Edit: Actually it's worst than I was thinking at first: when creating a new Launcher we were processing the arguments, then the method was switching by default the env noclasspath to false. So our changes in StandardEnvironment in #1936 had no impact for users.