feat(file_controller): Add support for to exclude file by adding them to .git/info/exclude

[gozes]

This PR will add support to use I to add a file the in the repo to .git/info/exclude file

Closes: #2014

[jesseduffield]

Thanks for making this @gozes. Instead of having two separate keybindings, I would instead stick to the original keybinding of lowercase 'i' but have it open a menu where the user can choose whether they want to 'ignore' the file or 'exclude' it. There's two three main reasons for this:

1. it frees up the 'I' keybinding for some other future use case
2. it's more extensible: for example, we may later want to add a third menu option to globally ignore a file.
3. it in effect adds a confirmation step to the ignore flow which I actually think is currently lacking, given that it's not super-easy to un-ignore something from within lazygit.

The menu itself can have a keybinding of 'i' for ignore and 'e' for exclude so that the lazygit pros can just type 'ie' to exclude a file and 'ii' to ignore.

We'll probably need to update an integration test accordingly but I can help out if you need for that.

Lemme know your thoughts @gozes @mjarkk

[gozes]

@jesseduffield I do think that having i and I maintains the internal constancy of how the file window operaters given that there's an established pattern in the file window of the lower case letter doing an action and the uppercase version doing the same action with a twist. For example, c and C to commit or commit with editor, or d and D to throw away changes.

What do you think think?

@mark2185 thoughts?

[mark2185]

I'm with @jesseduffield on this one, any chance of saving up keyboard real-estate is too good to pass.

Maybe there really are only two options, save to .gitignore or …/exclude, but maybe there's a third option we don't know about.

And maybe there's something that will fit I better down the road, but we don't yet know what it might be.

In short - I vote for having a menu.

[gozes]

Ok then menu it's. @mark2185 , can you or @jesseduffield point me at some part of the codebase where we do something similar so I can use it as a guide. :)

@jesseduffield would definitely appreciate some help with the integration test :)

[mark2185]

@gozes I can give you this, it's a PR that added a menu with two options, which is exactly what we need here :)

As for integration tests, the .gitignore one should work as expected because that file is tracked by git, but the exclude file is invisible to it, so that'll require some work, but we'll cross that bridge when we get to it.

To run the integration tests, go run test/lazyintegration/main.go will bring up everything you need.
Record a new test (or duplicate one and modify it) that opens the menu and adds something to .gitignore and then quit.

[gozes]

@mark2185 thanks for that :) I have a look when I get back to my laptop.

[gozes]

Ok got the menu working. Just traying to sort out the integration test now :)

[gozes]

@mark2185 @jesseduffield ok, Menu and its integration test done :)

[jesseduffield]

left some feedback. We should also have an integration test for excluding a file, and I'd also add a test for trying to exclude the .gitignore file (as I believe as this PR stands there's currently a bug around that due to checkTracking not returning a bool)

[jesseduffield]

Looking good, couple more things

[jesseduffield]

@gozes nice work. Code looks good, but we've got some failing CI steps

[gozes]

@gozes nice work. Code looks good, but we've got some failing CI steps

@jesseduffield I have fixed the linter and cheatsheet errors. However, I'm having a hard time debugging the falling test because if I run go run test/lazyintegration/main.go and then run the integration test I added they all pass OK for me however they seem to be falling in CI and I can't really tell why. Any idea what I should be looking at? I'm trying to see if there is something to do with the action itself but getting nowhere 😞

[gozes]

Ok, so I had a second look at this, aside from intentionally breaking the code to make the test fail, I can't really get them to fail locally. @mark2185 @jesseduffield any ideas what maybe be going on here? How come they fail in Ci but pass fine locally? I have missed something? feels like I have.

[jesseduffield]

@gozes When running these tests locally they're intermittently failing. When they fail, the snapshot they produce is actually a snapshot of the lazygit repo itself rather than the test repo. CI is doing the same thing. I'm not sure why that's happening but I'd try adding:

git config user.email "CI@example.com"
git config user.name "CI"

to each setup.sh file for the sake of consistency with other tests, and see if that does the trick. I can't see any other reason why the tests would be failing in the way they are.

[gozes]

@jesseduffield done, let hope that will fix it in ci :)

[gozes]

@jesseduffield nop look like that didn’t do the trick 😭

[mark2185]

@gozes By setting the user parameters before initializing the repo actually sets them for the parent repo, not for the newly created repo (submodule, basically).

Try moving said lines below git init

[jesseduffield]

@gozes dang. New theory: I've noticed that when we have a .gitignore file in a repo snapshot it actually applies to the parent lazygit repo. That is: we have myfile in our repo but it's ignored, but that actually means that when CI runs, it doesn't see myfile because it was never committed.

I've got a PR up to see if my theory is right: #2027

If that passes we can merge it all together

[jesseduffield]

Damn that didn't fix it. Still good to have anyway!

[jesseduffield]

Also that PR applies @mark2185 's suggestion

[mark2185]

Also that PR applies @mark2185 's suggestion

Any other ideas?

[jesseduffield]

Here's something:

The problem is that we're trying to take a snapshot of the 'actual' repo and the 'expected' repo but it seems like we don't actually have a .git directory in one of them so the snapshot ends up dealing with the entire parent lazygit repo. My current suspicion is that somewhere along the line something is failing but we're ignoring the error. BTW all this happens in pkg/integration/integration.go

BTW the .git_keep thing is actually the .git directory just renamed because git refuses to track any subdirectories named .git. There's a step to convert it from .git_keep to .git before taking the snapshot. Maybe that's failing.

[jesseduffield]

So turns out we actually did need to create an initial commit. Very strange. At any rate, my PR is passing now so @gozes I'm gonna squash your commits on that into one and then I'll merge it

[jesseduffield]

closing in favour of #2027 (though this PR did all the work ;) )

[jesseduffield]

Awesome work on your first PR @gozes !

[gozes]

@jesseduffield thanks. Looking forward to v0.35 release :)