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

Add root node in file tree #4346

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

stefanhaller
Copy link
Collaborator

  • PR Description

If files at the root level of the repository have changed, there was no way to see the combined diff of all of them. Fix this by inserting a <root> item in this case. This is useful in repositories such as git.git, which have all their .c files at root level (gasp).

If only files inside some folder have changed, the root item gets compressed away automatically, so nothing changes for that case (which I guess should be more common for most people).

This is a very rough draft. I'm not sure the implementation is the best one, and many tests fail. Just checking if we like the behavior before I go about adapting them all.

Addresses #4331.

  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@ratorx
Copy link

ratorx commented Mar 2, 2025

Can confirm this is working well for me (have no opinions about implementation :))

It solves part of my problem, but I'll put more details about that in the discussion.

@stefanhaller stefanhaller force-pushed the Add-root-node-in-file-tree branch from b5db34b to 36e48b0 Compare March 2, 2025 12:19
@stefanhaller
Copy link
Collaborator Author

I'm reasonably happy with the implementation now, and I cleaned up the commit history to make it reviewable. The distinction between GetPath and GetInternalPath might be a bit brittle and error-prone (easy to use the wrong one), but it's well-documented and the most reasonable solution I could think of. Let me know if you have any better ideas.

I wouldn't want to go about adapting all the tests (lots of them) before we have agreement about how the root item should look. Right now it's <root>, but does that look too ugly? Should it be just root? And does it have to be i18n-ized? I also considered making it just ., but maybe that's too subtle. But it would conceptually make sense; pressing space on it to stage all files would be equivalent to git add . on the command line.

@jesseduffield
Copy link
Owner

As discussed let's go with '/' for the root

@stefanhaller stefanhaller force-pushed the Add-root-node-in-file-tree branch from 36e48b0 to 99e56d8 Compare March 6, 2025 10:06
@stefanhaller
Copy link
Collaborator Author

As discussed let's go with '/' for the root

Ok, force-pushed with that change if you want to try it more. Still no progress on adapting the tests, will do that on the weekend, probably.

@jesseduffield
Copy link
Owner

Thanks, will do

@jesseduffield
Copy link
Owner

Funnily enough I just hit an index.lock error when staging all files via the root file. It appears this is not in fact equivalent to pressing 'a', and I wonder if we should just use an explicit git add . (or whatever we do when we press 'a') upon pressing space on the root file. It's possible that index.lock error was just a freak accident, but I also assume there's a performance difference between the two approaches.

@stefanhaller
Copy link
Collaborator Author

I do think the index.lock error is unrelated. I also get this (very occasionally) when hitting space on non-root items.

The difference between space on the root item and a is git add -- . for the former, and git add -A for the latter. I have to read up on what difference the -A makes, but when hitting space on a subfolder we also use git add -- <path>, so I don't see why it should be any different at the root.

@jesseduffield
Copy link
Owner

Oh okay, good to know. Happy to consider it a coincidence.

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