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

Enhancements to bash completions for better file/directory handling (take 2) #782

Closed
wants to merge 13 commits into from

Conversation

gtristan
Copy link

@gtristan gtristan commented May 8, 2017

This addresses the issues outlined in #780

  • Changes the bash entry point to use "complete -o nospace"
  • Adds abstract method to ParamType allowing the parameters to participate in the completion
  • Migrates Choice completion implementation into types.py on the actual Choice type
  • Implement the completions() method for BoolParamType, File and Path parameter types too

This behaves all around much better:

  • No more completing of filenames when that is undesirable
  • Completes filenames or directories selectively, depending on the parameter

Note: This is take two of pull request #781 ... I rebased my branch to have a more coherent history and did not expect github to delete the PR as a result, sorry.

gtristan added 9 commits May 6, 2017 19:16
Allows parameter types to implement their own completions.
The -o default will cause the shell to complete paths in any
case that the program did not provide completions in COMPREPLY,
this is undesirable as it will result in completing filenames
when the next argument or option in context is not a filename.

We use nospace here so we can control whether to append a space
or not to the completions, this will allow us to complete paths
with python code and allow completing more dynamically (so that
after completing a subdirectory there is no space appended and
one can continue to complete the next subdir).
…tions

Instead of special casing the Choice type.

Note that this checks the user provided 'autocompletions' list in
advance of consulting the parameter class, this should allow users
to override the default completion list of a parameter.

Some minor adjustments made to append a space to builtin completion
lists of commands and option names so that this works seamlessly
with the new "complete -o nospace" approach.
Slightly changed semantic for the autocompletion function which can
be specified in a decorator, now point out the semantic difference of
appending a space to a returned completion suggestion or not.

This should not be an API break since the autocompletion function
was added recently and has never been in a stable/official release.
After changing the bash wrapping script to invoke complete with the
nospace option, the decision to append a space and decide to start
completing the next program argument falls on click. So now all
results from get_choices() which are entirely "complete" and cannot
be further completed have an appended space.

Updated test case to expect this behavior from get_choices().
gtristan added 4 commits May 8, 2017 19:21
Since we access the filesystem for tab completion of files
and directories, we should be defensive and just ignore any
errors which might occur due to permission or other errors.
This was broken due to the way we split the input incomplete path
on the os separator token, when there is nothing to the left of
the last filesystem separator, we know we are trying to complete
from the root.

For windows, this should not happen since there will always
be a drive letter on the left of a valid terminating path separator.
We only want to filter out non-directories when searching for a
directory. We still want directory completions when searching for a file.
Ensure we still complete directory names when searching for a
regular file, we only want to disable non directory results when
searching for a directory path.
@gtristan
Copy link
Author

gtristan commented May 8, 2017

Addressed some corner cases regarding path name completions in some follow up commits instead of squashing those and rebasing the branch, as that seems to cause a merge request to be automatically closed.

I can rebase the branch locally so that these changes are squashed into the one commit implementing completions for file & directory paths, if that is desirable.

@davidism
Copy link
Member

Completion has changed significantly since this PR was posted. Would you or @stopthatcow review this to see if it's still needed and can be rebased?

@stopthatcow
Copy link
Contributor

stopthatcow commented Sep 14, 2018

There are definitely some attractive aspects of this PR. I like the idea of delegating some completion to the type, and path completion would be very nice to have. I can look into rebasing it on top of master this weekend, unless of course @gtristan wants to take a stab at it himself!

@gtristan
Copy link
Author

There are definitely some attractive aspects of this PR. I like the idea of delegating some completion to the type, and path completion would be very nice to have. I can look into rebasing it on top of master this weekend, unless of course @gtristan wants to take a stab at it himself!

It's great to see some activity on this front !

I would certainly love to upstream this stuff, as we're doing some pretty ugly hacks which reimplement the completions mechanism in order to get these features in BuildStream.

The most important part for us is the ability to perform partial completions, since we do completions of filenames which are intentionally not in the CWD, and soon we will be doing completions of entity names within a database/cache, these entities are also "pathy". If partial completions are not supported, then it is rather unacceptably slow (one would need to recursively list every leaf in an unpredictably large tree in order to report a full completion list in one go; for every <TAB>).

As far as I know, this can only be done by enabling -o nospace in the bash completion script; so that the program handling completions has the decision to append a space (fully complete) or not (partially complete, next <TAB> will show deeper results).

I'll have to refresh my memory on how compatible my proposed changes were, but I'm pretty confident that by delegating completions to the type and to delegated user functions; we can make sure that the behavior of allowing partial completions is opt in.

@jsmedmar
Copy link

jsmedmar commented Apr 10, 2019

@gtristan kudos to great work! Hope you can figure out this last bit so that we can see file completions in master. Again thanks so much for such thoughtful work in #780 and here.

@davidism
Copy link
Member

@gtristan thanks for working on this. I'm going to close it in favor of the updated work in #1403.

@davidism davidism closed this Feb 27, 2020
@davidism davidism added this to the 7.1 milestone Feb 27, 2020
@gtristan
Copy link
Author

@davidism Thanks for reviewing and for everyone who picked up my abandoned patches :)

I will try to find time to upgrade click in BuildStream and drop our custom code in favor of this :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f:completion feature: shell completion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants