Patch for user input validation when using commands 'Java: New Project...' and 'Java: New File from Template...'. #405
Conversation
oracle-contributor-agreement
bot
added
the
OCA Verified
|
@shivam71 can we also attach before-implementation and after-implementation ui screens image references with this pr description ? for future reference |
There was a problem hiding this comment.
For the time-being, while the upstream PR is still under review (mainly in the context of non-Java projects), these changes may still be considered for incorporation albeit as a draft patch (and with minor corrections included), since it has a positive impact on the user-experience. LGTM 👍
patches/7893-draft.diff
Outdated
| + return CompletableFuture.completedFuture(name); | ||
| + } | ||
| + } | ||
| + boolean isJavaTemplate = FileUtil.getMIMEType(template)!=null ? FileUtil.getMIMEType(template).equals("text/x-java"):false; |
There was a problem hiding this comment.
This maybe simplified to:
| + boolean isJavaTemplate = FileUtil.getMIMEType(template)!=null ? FileUtil.getMIMEType(template).equals("text/x-java"):false; | |
| + boolean isJavaTemplate = "text/x-java".equals(FileUtil.getMIMEType(template)); |
patches/7893-draft.diff
Outdated
| + } | ||
| + } | ||
| + boolean isJavaTemplate = FileUtil.getMIMEType(template)!=null ? FileUtil.getMIMEType(template).equals("text/x-java"):false; | ||
| + return isJavaTemplate ? client.showInputBox(new ShowInputBoxParams(Bundle.CTL_TemplateUI_SelectName(), desc.getProposedName())).thenCompose(new ValidateJavaObjectName()).thenApply(name -> builder.name(name)) : client.showInputBox(new ShowInputBoxParams(Bundle.CTL_TemplateUI_SelectName(), desc.getProposedName())).thenApply(name -> {return name != null ? builder.name(name) : null;}); |
There was a problem hiding this comment.
It may be less error-prone to convert this ternary condition to 3 separate statements with one intermediate conditional statement. That would maintain the common code in a common flow.
i.e.
Function<String, CompletionStage<String>> userInput = client.showInputBox(...);
if (isJavaTemplate) userInput = userInput.thenCompose(...);
return userInput.thenApply(...);
patches/7893-draft.diff
Outdated
| "CTL_TemplateUI_SelectName=Name of the object?", | ||
| "# {0} - path", | ||
| - "ERR_InvalidPath={0} isn't valid folder", | ||
| + "ERR_InvalidPath={0} isn't valid folder or is read only", |
There was a problem hiding this comment.
nit: add a before valid in this and the following error strings. E.g.:
| + "ERR_InvalidPath={0} isn't valid folder or is read only", | |
| + "ERR_InvalidPath={0} isn't a valid folder or is read-only", |
The ui steps are same I can add the screen shots of the error boxes that pop up in case of invalid input |
shivam71
force-pushed
the
user_input_validation_patch
branch
from
b546422 to
e70007b
Compare
|
Addressed the review comments |
…t...' and 'Java: New File from Template...'.
shivam71
force-pushed
the
user_input_validation_patch
branch
from
e70007b to
f19caf1
Compare
javax.lang.modelpackageSourceVersion.isNamemethod checked for validity of the package name and object name.