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

git add does not autocomplete files with changes #6650

Closed
flokno opened this issue Feb 24, 2020 · 14 comments
Closed

git add does not autocomplete files with changes #6650

flokno opened this issue Feb 24, 2020 · 14 comments
Labels
bug
Milestone

Comments

@flokno
Copy link

@flokno flokno commented Feb 24, 2020

On fish 3.1.0, Linux Mint 18.3 via the ppa:fish-shell/release-3 .

When I try to git add files with changes, fish doesn't do it, see this example where I try to add a file and git add only sees some unrelated untracked files:
https://asciinema.org/a/a4mpxoO8I0ok2wBYlQoj9JK2v

I played around with different config.fish, completions/git.fish etc., nothing helped. When I downgrade to 3.0.2-1511-gd903fe6 (randomly chosen because this is the version I use on a cluster), everything works as expected.

This is probably related to #5648 only for 3.1.0

@faho
Copy link
Member

@faho faho commented Feb 24, 2020

Works for me?

Please check that you're actually using the correct git.fish, with something like

for f in $fish_complete_path/git.fish; test -f $f; and echo $f; end

@flokno
Copy link
Author

@flokno flokno commented Feb 24, 2020

Thanks for commenting. The completions seem to be fine and I think they don't cause the problem. Is there a way to start fish without any configuration? I only found #1256

This is not super urgent, I switched back to 3.0.2 and it probably works fine for me. Let's see if someone else is affected.

@faho
Copy link
Member

@faho faho commented Feb 24, 2020

The completions seem to be fine and I think they don't cause the problem

It's either the completions not being fine or being overridden with ones that aren't fine.

What output does the command I posted give you?

Is there a way to start fish without any configuration?

XDG_CONFIG_HOME=(mktemp -d) fish

@flokno
Copy link
Author

@flokno flokno commented Feb 25, 2020

Thank you.

I get

for f in $fish_complete_path/git.fish; test -f $f; and echo $f; end 
~/.config/fish/completions/git.fish
/usr/share/fish/completions/git.fish

from my apt-get installed fish 3.1.0, and

~/.config/fish/completions/git.fish
/usr/local/share/fish/completions/git.fish

from my self built fish 3.0.2.

When running with XDG_CONFIG_HOME=(mktemp -d) /usr/bin/fish, I get

/usr/share/fish/completions/git.fish

only in fish 3.1.0, as expected.

I tried to replace the file in /usr/share with the one from /usr/local/share, to no effect -- git add <TAB> still suggests only untracked files for me.

@faho
Copy link
Member

@faho faho commented Feb 25, 2020

@flokno
Copy link
Author

@flokno flokno commented Feb 25, 2020

I tried this before. Tried again: no success.

@faho
Copy link
Member

@faho faho commented Feb 25, 2020

Ah, I think I got what's going on.

Linux Mint 18.3 is based on Ubuntu 16.04 "Xenial", which makes it quite old. It only features git 2.7.4, which is older than the 2.11.0 that introduced the "porcelain=2" format, so it only features the (rather awful) porcelain v1 format.

Something might be broken in the logic for that.

@faho
Copy link
Member

@faho faho commented Feb 25, 2020

Okay, this is caused by #6406.

We read a line like

 M CHANGELOG.md

into one variable, called line, with read -l line.

And because that uses the normal splitting mode (technically via $IFS), it will now try to be smart and remove whitespace before the last variable. And since we only use one variable, it will trim leading whitespace.

@krobelus? I don't think this really works as a fix for 6406, and I'm tempted to revert it and figure out something better.

@faho faho closed this as completed in cebfaa7 Feb 25, 2020
@faho faho added bug and removed needs more info labels Feb 25, 2020
@faho faho added this to the fish 3.1.1 milestone Feb 25, 2020
faho added a commit that referenced this issue Feb 25, 2020
Since #6406, read will trim whitespace before the last variable.

In this case there is only one variable, and the line looks like

 M CHANGELOG.md

so it does indeed start with whitespace, and the whitespace is quite
significant.

Fixes #6650.

[ci skip]
@faho
Copy link
Member

@faho faho commented Feb 25, 2020

Fixed the immediate problem in the git completion script, added to 3.1.1 as 298f43b.

@krobelus
Copy link
Member

@krobelus krobelus commented Feb 25, 2020

I'd say that relying on the behavior requested by #6406 does not feel too good,
and looking at this example, read line should obviously not strip whitespace.
However, bash also strips the whitespace here.

If I'm not mistaken the fix works for the examples given in the issue.
I don't really have a strong opinion on what the behavior should be; I barely ever use read (string split has a much more obvious interface). Compatibility with other shells seems like a safe choice.

I imagine trying to be yet smarter and not remove whitespace if there is exactly one variable would just be asking for more trouble 😈

@faho
Copy link
Member

@faho faho commented Feb 25, 2020

However, bash also strips the whitespace here.
[...]
Compatibility with other shells seems like a safe choice

Note that bash's (well, POSIX's) read is bad. The canonical answer to read a file line-by-line is

while IFS= read -r aline

Let's try to improve on that!

If we can do something to let it just be

while read aline

we should probably do it.

I imagine trying to be yet smarter and not remove whitespace if there is exactly one variable

I've thought of checking whether the last variable is also the first, yes. I'm not entirely sure on that? Phrasing it that way feels better than "exactly one variable", and it would fix this case, but at a glance it still looks a bit dirty.

@krobelus
Copy link
Member

@krobelus krobelus commented Feb 25, 2020

Yeah, improving read sounds good if we can pull it off. I don't know how it is being used out there.
Anyway, reverting 1410f93 would be the first step, I guess it's not a very important feature.

@krobelus
Copy link
Member

@krobelus krobelus commented Feb 28, 2020

Let's revert 1410f93, I think it does more harm than good.
@zanchey perhaps you could still squeeze the revert commit in 3.1.1?

krobelus added a commit that referenced this issue Feb 29, 2020
krobelus added a commit to krobelus/fish-shell that referenced this issue Feb 29, 2020
See fish-shell#6650.

This reverts commit 1410f93.

(cherry-picked from commmit 91fcb8c)
@zanchey
Copy link
Member

@zanchey zanchey commented Mar 7, 2020

In 3.1.1 as 0c74ff4 with thanks to @krobelus

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug
Projects
None yet
Development

No branches or pull requests

4 participants