#836: fix exit autocompletion#1950
Conversation
|
|
Coverage Report for CI Build 26149130916Warning No base build found for commit Coverage: 70.947%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
| line = line.trim(); | ||
| if (line.equals("exit")) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This is not the completion but the actual functionality that exit works as command and actually ends the shell.
There was a problem hiding this comment.
Thanks. I restored/kept this part in ShellCommandlet since it is the actual shell exit handling and not part of the completion logic.
| if (wordIndex == 0 && !currentWord.isEmpty() && "exit".startsWith(currentWord)) { | ||
| candidates.add(new Candidate("exit")); | ||
| } |
There was a problem hiding this comment.
You fix looks good but why did you not simply put this fix inside IdeCompleter instead of building an anonymous wrapper class around it?
IMHO you can also return after completing exit (and not continue with the generic completion).
Finally, I would suggest to create a constant for "exit" this is required 3x in this code.
There was a problem hiding this comment.
Thanks, I removed the anonymous completer wrapper, moved the exit completion logic into IdeCompleter, and added an EXIT_COMMAND constant.
I kept the generic completion running after adding the exit candidate so that possible commandlets with the same prefix are not skipped.
- restored exit command handling since it is actual shell functionality - removed anonymous completer wrapper - moved exit completion logic into IdeCompleter - added EXIT_COMMAND constant
Caylipp
force-pushed
the
feature/836-fix-exit-autocomplete
branch
from
d7b4d41 to
d09cc84
Compare
5 participants
This PR fixes #836
Implemented changes:
How to Test:
Run in Git Bash:
mvn -pl cli exec:java -Dexec.mainClass=com.devonfw.tools.ide.cli.Ideasy -Dexec.args="shell"Then verify:
e<TAB>-> suggestsexit<TAB>with empty input -> noexitz<TAB>-> noexitintellij e<TAB>) -> noexitChecklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal