Skip to content

Make configure fall back to bashdefault and default #503

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

Closed
wants to merge 1 commit into from

Conversation

villevoutilainen
Copy link

This is necessary for some configure scripts where the --help
of the configure doesn't describe option arguments accurately.
Qt, for instance, has a configure script where -android-sdk is followed
by a path. The path is not completed because this script doesn't know
how to do that, and this fallback makes it work. It seems reasonable
in general to fall back on default completions if option completions
didn't work.

This is necessary for some configure scripts where the --help
of the configure doesn't describe option arguments accurately.
Qt, for instance, has a configure script where -android-sdk is followed
by a path. The path is not completed because this script doesn't know
how to do that, and this fallback makes it work. It seems reasonable
in general to fall back on default completions if option completions
didn't work.
@villevoutilainen
Copy link
Author

Those check failures seem completely borked, is there a way to perhaps re-run them once the test environment is not failing catastrophically?

@@ -37,6 +37,6 @@ _configure()
[[ ${COMPREPLY-} == *= ]] && compopt -o nospace
fi
} &&
complete -F _configure configure
complete -o bashdefault -o default -F _configure configure
Copy link
Owner

Choose a reason for hiding this comment

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

While I agree with:

It seems reasonable in general to fall back on default completions if option completions didn't work.

...for this particular completion, the problem with the above implementation is that we don't have a way to say what worked or not. For example, we explicitly want to prevent all completions following --help, --version, --program-suffix and friends, but if we slap the defaults in the mix, our prevention will no longer work. I wish there was a way to signal success/not someway to prevent the defaults from kicking in (e.g. via some return value), but I'm not aware of a way.

Copy link
Author

Choose a reason for hiding this comment

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

I would rather have possible superfluous completions after --help than have the completion of a directory not actually complete after an option that the completion doesn't recognize, but wants a directory anyway. The former is just a nuisance, the latter leads to copypaste typos that instead of failing the configure itself will fail the subsequent build.

Copy link
Owner

Choose a reason for hiding this comment

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

Mileages vary. Anyway I think it's possible to accomplish the original intent without sacrificing non-completions by not touching complete options at all, but doing this:

--- a/completions/configure
+++ b/completions/configure
@@ -25 +25,4 @@ _configure()
-    [[ $cur != -* ]] && return
+    if [[ $cur != -* ]]; then
+        _filedir
+        return
+    fi

Copy link
Author

Choose a reason for hiding this comment

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

I verified that that works; plz patch that in. :)

@scop
Copy link
Owner

scop commented Mar 11, 2021

Those check failures seem completely borked

Yeah, I've been kind of hoping for them to fix themselves, because I believe they also appeared out of the blue without us doing anything to provoke them. Help welcome if there are ideas though. (It's not limited to Alpine, nor caused by an upgrade of pytest to a fresh version.)

@scop scop closed this in 34d040b Mar 16, 2021
@scop
Copy link
Owner

scop commented Mar 16, 2021

Done, slightly revised.

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.

2 participants