Fixing scriptDir and scriptOutputDir #579
Conversation
| return argMap.containsKey(SCRIPT) | ||
| || argMap.containsKey(SCRIPT_OUTPUT_DIR) | ||
| || argMap.containsKey(SCRIPT_DIR) | ||
| || argMap.containsKey(SCRIPT_OUTPUT_FILE); |
There was a problem hiding this comment.
This seems like only SCRIPT and SCRIPT_DIR should be checked here.
The hasScripts() is used to enable the noServer option by default so it seems right when it would not be affected by the output dir argument.
| case SCRIPT_DIR: | ||
| { | ||
| if (!p.isEmpty()) { | ||
| settings.setOutputDir(p); |
There was a problem hiding this comment.
This does not look right - it should rather be setScriptDir(p). In fact, get/setOuptutDir() should be removed as it is replaced by get/setScriptOutputDir() (you would need to check where getOutputDir() is used and replace it with getScriptOutputDir())
jbachorik
left a comment
jbachorik
left a comment
There was a problem hiding this comment.
Hi, thanks for looking into this problem.
There are just a few things that needs to be cleared up before I can approve.
|
Hi @joachimhs , I wonder - will you be able to finish this PR? |
|
I am sorry. There was no activity on this PR for a very long time - I am going to address the problem in #593 and add an attribution to you, Joachim, there. I hope it is ok. |
Properly handle script arguments. Based on #579 authored by @joachimhs
2 participants
Adding scriptOutputDir, and renaming the old SCRIPT_OUTPUT_DIR to SCRIPT_DIR.