Skip to content

Conversation

@DannyBen
Copy link
Member

@DannyBen DannyBen commented May 20, 2024

Raised and discussed in #66

This PR temporarily (locally) removes three characters from the list of word break characters: colon, semicolon, equal sign.

Copy link
Contributor

@kronn kronn left a comment

Choose a reason for hiding this comment

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

I think I need to add these approvals to my setup for scripts, looks really neat.

Changewise, I like that the change is sensibly small, local and reverted after its use.

@DannyBen
Copy link
Member Author

I think I need to add these approvals to my setup for scripts, looks really neat.

rspec_approvals is a massive time saver for my tests, even more so in command line utilities.

I will merge. Release will be coming later this week (you can use the edge in the meantime).

@DannyBen DannyBen merged commit c65bb2e into master May 21, 2024
@DannyBen DannyBen deleted the fix/completion-wordbreaks branch May 21, 2024 05:05
@kronn
Copy link
Contributor

kronn commented May 21, 2024

It seems like the locally set env-var does not work as intended. With the generated script, I did not get the intended completion-behaviour. Maybe hold off on a release, I'd like to try to fix this today or tomorrow.

@DannyBen
Copy link
Member Author

DannyBen commented May 21, 2024

You are right. I think that disabling it broke it.
Placing it right before calling complete:

COMP_WORDBREAKS="${COMP_WORDBREAKS//[:=]/}" complete -F _asd_completions asd

works, but seems to affect the global state.

EDIT: Does not work.

@kronn
Copy link
Contributor

kronn commented May 21, 2024

npm had a similar problem: npm/npm@d7271b8

I thought that the setting inside the function would be the elegant and correct solution. That's what I get for thinking, I guess...

@DannyBen
Copy link
Member Author

npm had a similar problem: npm/npm@d7271b8

I see that they reverted?
Setting the environment variable globally is definitely not a solution, due to the sourced nature of completion scripts.

There must be a way to change this temporarily - did you follow the npm thread to see if they solved it in a different way? Or did they just revert and left it unsolved?

I was unable to get any better solutions from any of the chat AIs.

@kronn
Copy link
Contributor

kronn commented May 21, 2024

They reverted the way with the env-var, not their custom parsing: https://github.com/npm/cli/blob/latest/lib/utils/completion.sh

I'll try and see if I can get it to work similarly.

@DannyBen
Copy link
Member Author

Should I revert this PR, or are you trying to make it work?

@kronn
Copy link
Contributor

kronn commented May 24, 2024

Both. :-)
Sorry for the late repsonse.

If I get to a solution, I'll send a PR.

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.

3 participants