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

__fish_git_prompt_informative_status should obey showUntrackedFiles=false #4842

Closed
snnw opened this issue Mar 22, 2018 · 9 comments
Closed

Comments

@snnw
Copy link
Contributor

snnw commented Mar 22, 2018

In a large git repository with many untracked files, git ls-files --others takes a long time to run.

The non-informative prompt uses __fish_git_prompt_showuntrackedfiles and bash.showUntrackedFiles to disable the untracked files part of the prompt, however, the informative prompt always tries to determine if there are untracked files.

It would be great if this wasn't inconsistent. 😃

@ridiculousfish ridiculousfish added this to the fish-future milestone Mar 24, 2018
@ridiculousfish
Copy link
Member

ridiculousfish commented Mar 24, 2018

Thanks for filing, I'll be happy to merge a PR if you feel so inclined!

@faho
Copy link
Member

faho commented Mar 25, 2018

In a large git repository with many untracked files, git ls-files --others takes a long time to run.

Ideally, this would also use git status --porcelain, like the git completions do since ea897fc (and a switch to the v1 format with 4fac2f9).

We've found that to be a bit faster, and it would also do the job of the git diff --name-status calls before.

@snnw
Copy link
Contributor Author

snnw commented Mar 26, 2018

@faho I like it! git status --porcelain already does the right thing (tm) for me, since I have status.showuntrackedfiles=no in .git/config for this repository.

Ideally, one would use the same __fish_git_files as the completions, but I don't know if that function is (always) accessible from __fish_prompt. Could it be moved to a shared location?

@faho
Copy link
Member

faho commented Mar 26, 2018

git status --porcelain already does the right thing (tm) for me, since I have status.showuntrackedfiles=no in .git/config for this repository.

Does it? That would actually mean that git status --porcelain is slightly broken - in some cases we absolutely want untracked files, no matter what your normal config says! E.g. when you complete git add .

Ideally, one would use the same __fish_git_files as the completions, but I don't know if that function is (always) accessible from __fish_prompt.

It's not. Only if you have executed the git completions beforehand.

Could it be moved to a shared location?

Weeeeelllll... the problem is getting the data out of the function without calling it multiple times. Obviously calling set -l untracked_files (__fish_git_files untracked); set -l addedfiles (__fish_git_files added) isn't going to be fast enough.

We need the number of different kinds of files, and for that we'd either need to add an option to __fish_git_files to do the counting (and then print the numbers in a specific order, so we could go set -l counts (__fish_git_files); set -l untracked_count $counts[5] or so), or we'd need to get it to print markers between the files....

Or we'd need to actually duplicate the code.

(This is one of those cases where an "upvar" mechanism - making it possible to define local variables in the outer function - would be useful. Or some other way to pass structured data)


@snnw: For testing purposes - do you have a nice public repo where the build process or something creates loads of untracked files?

@snnw
Copy link
Contributor Author

snnw commented Mar 26, 2018

git status --porcelain already does the right thing (tm) for me, since I have status.showuntrackedfiles=no in .git/config for this repository.

Does it? That would actually mean that git status --porcelain is slightly broken - in some cases we absolutely want untracked files, no matter what your normal config says! E.g. when you complete git add.

Well, that explains why fish wouldn't complete git add in this particular repository! I'm not sure if this is a feature or a bug on git status --porcelain's part, seeing as it does prevent a lengthy walk of the untracked files. Maybe it makes sense to prompt the user whether they really want to see git add completions when status.showuntrackedfiles=no. That would've helped, for me at least, in figuring out why it wasn't doing that and why it makes sense. (In this case, one probably has to revert back to git ls-files again...)


Re code-sharing, I see. Complicate the code to share more vs code duplication...

Looking at __fish_git_files again, something like

set -l number_untracked (echo $git_files | grep (_ "Untracked file") | wc -l)
set -l number_added (echo $git_files | grep (_ "Added file") | wc -l)

would work, although it doesn't look great ☹️


On the testing front, the directory I'm seeing this on is my hopelessly cluttered $HOME, which I'm very slowly trying to move into several git repositories. Not something I can share, I'm afraid.

@faho
Copy link
Member

faho commented Mar 26, 2018

Well, that explains why fish wouldn't complete git add in this particular repository! I'm not sure if this is a feature or a bug on git status --porcelain's part

I just checked the git-status man page again, and it's not a bug, it's intended behavior.

-u[], --untracked-files[=]
Show untracked files.
[...]

The default can be changed using the status.showUntrackedFiles configuration variable documented in git-config(1).

So we have to add --untracked-files=normal, or maybe even all (or we'd use the current token if there is one).

Looking at __fish_git_files again, something like

Yeah, that's also an idea. We'd have to be careful to only match the translated description at the end, and we'd have to be careful not to call the translation too much (that actually calls gettext, which can be slow).

On the testing front, the directory I'm seeing this on is my hopelessly cluttered $HOME, which I'm very slowly trying to move into several git repositories. Not something I can share, I'm afraid.

Also not something you need to share 😄 - the idea is easily recreated. I just ran git init in my $HOME, so every single file in that is now untracked. That's a total of ~800k files+directories.

The news is git ls-files --others and git status --porcelain -uall are slow as molasses (>30s), while git status --porcelain -unormal takes 2s the first time, and 12ms after that (caching is great). So obviously the issue is decending into all those directories to enumerate all the files.

Presumably changing from getting all untracked files to getting untracked files in the current directory and directories would change the numbers, but I don't think that's all that bad - it might even be more like you'd expect (e.g. if we didn't have the "pcre2" directory added, that would be ~200 files - which is not really useful information). I would even imagine the most useful part of knowing the number of untracked files is just knowing if there are any at all.

@snnw
Copy link
Contributor Author

snnw commented Mar 26, 2018

Can confirm that git status --porcelain -unormal is comparatively very fast!

Going back to the original issue, namely, __fish_git_prompt. It only needs to know whether there are any untracked files to put its ... in the prompt, so -unormal is, like you say, entirely adequate for that.

Also, I wonder if -unormal can be made to work for git add as well, by completing just one level at a time, building the completion incrementally and only running git status --porcelain -unormal in the directory-entered-so-far. (But maybe I'm asking too much from the completion code 😄)

@faho
Copy link
Member

faho commented Mar 26, 2018

Also, I wonder if -unormal can be made to work for git add as well, by completing just one level at a time, building the completion incrementally

That was what I meant by "use the current token". It's possible, just a bit fiddly. I think we have something similar elsewhere already.

Going back to the original issue, namely, __fish_git_prompt. It only needs to know whether there are any untracked files to put its ... in the prompt, so -unormal is, like you say, entirely adequate for that.

I think the informative version prints the actual count, which would change. But as I said that probably doesn't really matter.

@zanchey
Copy link
Member

zanchey commented Feb 4, 2019

I think this is fixed by 15cf45a?

@faho faho closed this as completed Feb 4, 2019
@faho faho modified the milestones: fish-future, fish 3.1.0 Feb 4, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants