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

backward-kill-path-component is too aggressive with @ #6258

Closed
mqudsi opened this issue Oct 28, 2019 · 5 comments · Fixed by #7872
Closed

backward-kill-path-component is too aggressive with @ #6258

mqudsi opened this issue Oct 28, 2019 · 5 comments · Fixed by #7872
Labels
bug Something that's not working as intended
Milestone

Comments

@mqudsi
Copy link
Contributor

mqudsi commented Oct 28, 2019

With the cursor at the end of the following prompt:

~> dig example.com @127.0.0.1

backward-kill-path-component triggered once removes the 127.0.0.1:

~> dig example.com @

but when triggered a second time results in the following:

~> dig 

(Expected: ~> dig example.com )

@zanchey zanchey added the bug Something that's not working as intended label Oct 30, 2019
@zanchey zanchey added this to the fish-future milestone Oct 30, 2019
@Caroleq
Copy link
Contributor

Caroleq commented Mar 25, 2021

I'll dig it up. Is it a bug or actually expected behavior?

By moving from ~> dig example.com @ to ~> dig we (citing documentation, https://fishshell.com/docs/current/cmds/bind.html) move one path component (here example.com) to the left of the cursor which is expected.

By moving from ~> dig example.com @ to ~> dig example.com we wouldn't consume any path component - @ is not a component according to docs, it's a separator.

There is a test for similar case in https://github.com/fish-shell/fish-shell/blob/master/src/fish_tests.cpp, line 2721.

L"^echo /^foo/^bar{^aaa,^bbb,^ccc}^bak/^"

Moving from ~> echo / to ~> is the same as case as moving from ~> dig example.com @ to ~> dig .

But I also noticed that if separator is followed by spaces, triggering backward-kill-path-component consumes chars only up to separators.
For example:

~> dig example.com @<space>

after triggering backward-kill-path-component produces:

~> dig example.com 

From my understanding of the docs I would assume that no path component was consumed, since spaces and @ are not path component parts.

@ridiculousfish
Copy link
Member

I think it's a real bug. backward-kill-path-component should never delete from two different arguments.

@mqudsi
Copy link
Contributor Author

mqudsi commented Mar 26, 2021

That was my line of thinking as well.

@Caroleq
Copy link
Contributor

Caroleq commented Mar 26, 2021

So just to make sure, backward-kill-path-component should stop in the following positions ('^' are the positions to stop):

L"^echo ^/^foo^/^bar^{^aaa^,^bbb^,^ccc^}^bak^/^"

instead of current:

L"^echo /^foo/^bar{^aaa,^bbb,^ccc}^bak/^"

and

L"^echo ^hi ^> ^/^dev^/^null^"

instead of:

L"^echo ^hi ^> /^dev/^null^"

?
I would take up this issue.

@krobelus
Copy link
Member

should stop in the following positions

echo ^bak^/^

No, I think this should still remove one "path component" (not only a separator) in the common case:

echo ^bak/^

So maybe the change is just:
"If there is a separator directly left of the cursor, consume all separators and a word."

Consuming multiple separators is current behavior, which is not affected by this issue:

echo ^bak ^///^

(I think we can still treat @ and / the same way, both are equally affected.)

@zanchey zanchey linked a pull request Mar 28, 2021 that will close this issue
3 tasks
@krobelus krobelus modified the milestones: fish-future, fish 3.3.0 Mar 29, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants