create dot file #29
create dot file #29
Conversation
There was a problem hiding this comment.
Thanks for this work!
An initial round of code review was made.
Also, it would be better to move this PR against your master branch eisop/ashley-master in the eisop repo (as currently this isn't a mature feature ready to be merged in the master branch).
Cheers,
Charles
| @@ -34,7 +34,7 @@ David Pritchard ([email protected]), created May 2013 | |||
|
|
|||
| String mainClass; | |||
| String exceptionMsg; | |||
| List<String> checkerOptionsList; | |||
| ArrayList<String> checkerOptionsList; //need a mutable list and add more flags if cfg enabled | |||
There was a problem hiding this comment.
Generally, more general declaration is better. Thats means, here is better to declare as List<String>, and you just initialize it by a ArrayList instance later on. So please revert this change.
| @@ -105,6 +129,7 @@ protected boolean initCheckerArgs(JsonObject optionsObject) { | |||
|
|
|||
| // figure out the class name, then compile and run main([]) | |||
| InMemory(JsonObject frontend_data, String enabled_cf, Printer checkerPrinter) { | |||
| // System.out.println("\ntry calling inMemory\n"); | |||
There was a problem hiding this comment.
Remove this debug print.
| this.exceptionMsg = "Error: Cannot setup directory CFG"; | ||
| return false; | ||
| } | ||
| if(! setupDir(new File("..//CFG//bin"))){ |
There was a problem hiding this comment.
I think InMemory class will compile the source code in memory, which means it doesn't need a bin dir to hold the byte code output. Could you try invoking checker-framework without indicating -d argument?
| return false; | ||
| } | ||
|
|
||
| this.checkerOptionsList.add("-d"); |
There was a problem hiding this comment.
Same comment as above, could you try remove this -d argument?
| } | ||
|
|
||
| this.checkerOptionsList.add("-d"); | ||
| this.checkerOptionsList.add("../CFG/bin"); |
There was a problem hiding this comment.
Same comment. Try remove bin dir.
| this.exceptionMsg = "Error: Cannot setup directory CFG/bin"; | ||
| return false; | ||
| } | ||
| if(! setupDir(new File("..//CFG//dotfiles_default"))){ |
There was a problem hiding this comment.
DOTCFGVisualizer will create an output dir indicated in outdir argument, so there is no need to check and create this dir.
| if (optionsObject.getBoolean("has_cfg")) { | ||
|
|
||
| //ugly, how can this be improved? | ||
| if(! setupDir(new File("../CFG"))){ |
There was a problem hiding this comment.
As we discussed before, at current stage you can ignore considering the parallel multi-users call conflicts and focus on implementing the main functions. So as a temporary workaround, we can just manually create a CFG directory in the server and here just assume there should have a CFG directory in the expected location.
| @@ -153,4 +179,20 @@ protected boolean initCheckerArgs(JsonObject optionsObject) { | |||
| this.checkerPrinter.printDiagnosticReport(diagnosticList); | |||
| } | |||
| } | |||
|
|
|||
| //create a directory, if exists already, delete and recreate a directory | |||
| protected static boolean setupDir(File dir) { | |||
There was a problem hiding this comment.
Maybe we don't need this method. As we can just assume there will be a CFG directory at the moment.
| @@ -34,7 +34,7 @@ David Pritchard ([email protected]), created May 2013 | |||
|
|
|||
| String mainClass; | |||
| String exceptionMsg; | |||
| List<String> checkerOptionsList; | |||
| List<String> checkerOptionsList; | |||
|
|
||
| //assume exists CFG directory | ||
| this.checkerOptionsList.add("-classpath"); | ||
| this.checkerOptionsList.add(this.CHECKER_FRAMEWORK + "/checker/dist/checker.jar"); |
There was a problem hiding this comment.
Isn't it a bit surprising that the checker.jar file is only needed when that flag is given? You also need it when normally running the processor. If this is indeed needed, can you add a comment documenting why?
|
Closed as this PR is not ready to merge into |
can successful create dot files (default)
to be improved: