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(__load_completion): do not warn when completing . first time #776

Merged
merged 1 commit into from
Aug 20, 2022

Conversation

scop
Copy link
Owner

@scop scop commented Jul 22, 2022

ca361be enables warnings about trying to source directories when loading completions. That's useful, but triggers the errors in the common case of trying to source something, . Tab

Special case . (and for completeness, even though not that interesting, ..) so we don't issue the warning about them.

@akinomyoga
Copy link
Collaborator

I have a naive question. Do we need to output warnings here? Maybe we can just ignore the directories because the warning message disturbs the cursor positions in the terminal.

@scop
Copy link
Owner Author

scop commented Aug 20, 2022

#506 has the discussion related to this.

I'm not that concerned about messing with the output; this is a broken setup that should be fixed and not masked, consistent with what happens with a completion file with invalid syntax, or an unreadable one.

@akinomyoga
Copy link
Collaborator

Thank you for your explanation. Then, that's fine to keep the error message.

Maybe we can also explicitly mention in the error message that putting subdirectories in completions directories is the wrong setup?

bash_completion Show resolved Hide resolved
@scop
Copy link
Owner Author

scop commented Aug 20, 2022

As mentioned in the commit message, the current warning mimics what bash itself would emit when sourcing a directory. If we keep it that way, there will be minimal behavioral changes ahead for users when the day comes we can drop our own -d check (probably when we start to require bash >= 4.3 -- would be good to find out the exact version in a comment here and document it, https://bugzilla.redhat.com/show_bug.cgi?id=903833)

ca361be enables warnings about trying
to source directories when loading completions. That's useful, but
triggers the errors in the common case of trying to source something,
`. <TAB>`.

Special case `.` (and for completeness, even though not that
interesting, `..`) so we don't issue the warning about them.

Closes #703
@akinomyoga
Copy link
Collaborator

Thank you! I see the background! Please merge it as is.

@scop scop merged commit 017fb55 into master Aug 20, 2022
@scop scop deleted the fix/load-completion-dot branch August 20, 2022 05:51
@scop
Copy link
Owner Author

scop commented Aug 20, 2022

Done, along with 16ce83f documenting the bash version needing the check (verified locally, didn't bother to check 4.2.x patch versions).

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