- 
                Notifications
    You must be signed in to change notification settings 
- Fork 399
fix(java,rsync,ssh): complete syntactically incomplete cur #1485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|  | ||
| local REPLY | ||
| _comp_dequote "$cur" | ||
| _comp_dequote_incomplete "$cur" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not immediately clear to me why we'd do this when completing java packages, the commit messages nor the referenced issues don't mention it either, and there's seemingly no test suite coverage for it. Is it necessary, and if yes, can we somehow clarify why and add test cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With javadoc 'a[tab], cur becomes an incomplete word cur="'a". If we use _comp_dequote, nothing is completed because modules starting with 'a are not found. In dealing with a value coming from cur, we need to use _comp_dequote_incomplete in general.
- I rebased on top of the latest main, and I added a test case intest/t/test_javadoc.py.
- I described this in the commit message of the first commit.
- I mentioned it in the code documentation of _comp_dequoteand_comp_dequote_incomplete.
ce45cc9    to
    3c28b9b      
    Compare
  
    scop#1255 (comment) `cur` is the word that is currently input by the user and can be syntactically incomplete when the completion is requested. For example, it is typical to attempt a completion with an opening quotation `'` but without a closing quotation: $ javadoc 'a[tab] Or, if there are two candidates `a\ b.txt` and `a\\\ c.txt`, the first attempt of the completion would insert the common part `a\`, where the escape target of `\` is missing. To handle these cases, in dequoting values coming from `cur`, we need to deal with the cases with incomplete values. This patch adds a new utility `_comp_dequote_incomplete` and replace `_comp_dequote` with it everywhere `_comp_dequote` is used for values coming from `cur`. Co-authored-by: Yedaya Katsman <[email protected]>
3c28b9b    to
    455f8c1      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed in the review and already merged, but I think this is a filename that will cause problems e.g. on Windows, and it would be better created dynamically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also separated from #1357. See the commit messages. This fixes #1255.