Fix nodejs-server path issue in windows platform #7808
Conversation
| @@ -164,7 +164,7 @@ public String apiFilename(String templateName, String tag) { | |||
| } | |||
|
|
|||
| private String implFileFolder(String output) { | |||
| return outputFolder + "/" + output + "/" + apiPackage().replace('.', '/'); | |||
| return (outputFolder + File.separator + output + File.separator + apiPackage()).replace('.', File.separatorChar); | |||
There was a problem hiding this comment.
- Is there a file path API for Java to avoid such an error-prone path concatenation implementation?
- Why should all the dots (
.) be replaced with a path separator? What if I have a folder with a dot in its name?
There was a problem hiding this comment.
-
Not that I'm aware of and I agree the current solution is error-prone so I'm open to new ideas for a better implementation.
-
I do not know the reason. The logic was added before I joined this project. My suggestion is to keep the current behavior and try to come up with workaround/solution if users file an issue about it.
There was a problem hiding this comment.
-
OK
-
If you want to keep the current behavior, you should probably drop those parentheses to limit the replacement only for the
apiPackage()string instead of the whole result:outputFolder + File.separator + output + File.separator + apiPackage().replace('.', File.separatorChar)
|
LGTM |
There was a problem hiding this comment.
Doesn't work on Windows
Change 1
NodeJSServerCodegen.java line 88: back to
supportingFiles.add(new SupportingFile("writer.mustache", ("utils").replace(".", "/"), "writer.js"));change 2 : NodeJSServerCodegen.java, line 172
Use this :
String replacement = (java.io.File.separator.equals("\\") ? java.io.File.separator + java.io.File.separator : java.io.File.separator)
+ implFolder +
(java.io.File.separator.equals("\\") ? java.io.File.separator + java.io.File.separator : java.io.File.separator) ;
|
@dlarr thanks for the review. I think my approach should still work. Given that it does not impact Mac/Linux, I'll merge it and ask my friends to try it in their Windows box. |
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\.3.0.0branch for changes related to OpenAPI spec 3.0. Default:master.Description of the PR
To fix #6443
cc @CodeNinjai (2017/07) @frol (2017/07) @cliffano (2017/07)