Skip to content
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

fix(python): complete filenames for script arguments #1018

Merged
merged 2 commits into from
Aug 7, 2023

Conversation

akinomyoga
Copy link
Collaborator

Ref. ohmybash/oh-my-bash#462

When [-c command | -m module | file] is already specified before cword, the current implementation of the python completer tries to complete filenames as arguments to the command/module/script. However, this doesn't work after an option of the form -?. The following is a case reported in ohmybash/oh-my-bash#462:

python some_script.py -p [TAB]

The current implementation seems to try to detect the situation by checking the previous and the second previous arguments, but we should explicitly test if [-c command | -m module | file] is already specified.

Note: Removing an old case pattern that excludes python and -?

I think this is actually related to the following line that we once discussed in #952 (see replies [1,2]):

!(?(*/)?(micro)python*([0-9.])|?(*/)py@(py|ston)*([0-9.])|-?))
if [[ $cword -lt 2 || ${words[cword - 2]} != -[QWX] ]]; then
_comp_compgen_filedir
return
fi

After looking at this code again, I now believe this piece of code tried to detect whether the current word is an argument just after [-c command | -m module | file] or not. The case pattern and the if statement exclude the case where

  1. the previous argument is the command name (?(*/)?(micro)python*([0-9.])|?(*/)py@(py|ston)*([0-9.])),
  2. the previous argument is an option (-?), or
  3. the second previous and previous arguments form a pair of an option plus an option argument (${words[cword - 2]} != -[QWX]).

The remaining situation is likely to be only the case where the previous argument is a command/module/script of [-c command | -m module | file]. Since we now explicitly check [-c command | -m module | file], we should remove this heuristic detection of the first argument.

Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. This is a bit of a corner case maybe, could we add some test cases to cover/guard the intended behavior?

@akinomyoga akinomyoga force-pushed the python-args branch 4 times, most recently from ab93f09 to a1ba42a Compare August 6, 2023 18:51
@akinomyoga
Copy link
Collaborator Author

I added small adjustments and tests.

test/t/test_python.py Outdated Show resolved Hide resolved
@akinomyoga akinomyoga force-pushed the python-args branch 2 times, most recently from 82b9b05 to 69b6030 Compare August 6, 2023 23:11
@scop scop merged commit 397a49d into scop:master Aug 7, 2023
3 of 7 checks passed
@scop
Copy link
Owner

scop commented Aug 7, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants