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

Improve git icons on directories #1809

Merged
merged 16 commits into from
Dec 17, 2022
Merged

Improve git icons on directories #1809

merged 16 commits into from
Dec 17, 2022

Conversation

chomosuke
Copy link
Collaborator

@chomosuke chomosuke commented Dec 6, 2022

After adding option git.show_on_open_dirs, I've discovered some undesirable behavior:

  • ignored directory's ignored status going away after opening it.
  • having no way of knowing a file a deleted once the directory it's in is opened.

This PR fixes the above two short coming and also made parent directories show all the git status icons from their children.

The ordering of the git icons in lua/nvim-tree/renderer/components/git.lua:10 are made to preserve the original ordering of some of the multi-icons status. I'd be happy to change it if anyone have different preferences :).

@alex-courtis
Copy link
Member

The ordering of the git icons in lua/nvim-tree/renderer/components/git.lua:10 are made to preserve the original ordering of some of the multi-icons status. I'd be happy to change it if anyone have different preferences :).

I think we can remove that. The order of statuses to icons retrieved by git.get_icons is Good Enough and easy to change following its normalisation.

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested a variety of cases, however it's not practical to test all cases.

Please:

Thank you so much: this refactor and consolidation is a great improvement to the codebase.

doc/nvim-tree-lua.txt Outdated Show resolved Hide resolved
@@ -48,7 +45,7 @@ function M.reload_explorer()
end

function M.reload_git()
if not core.get_explorer() or not git.config.enable or event_running then
if not core.get_explorer() or not git.config.git.enable or event_running then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

lua/nvim-tree/explorer/common.lua Outdated Show resolved Hide resolved
lua/nvim-tree/renderer/components/git.lua Outdated Show resolved Hide resolved
@chomosuke
Copy link
Collaborator Author

chomosuke commented Dec 12, 2022

The ordering of the git icons in lua/nvim-tree/renderer/components/git.lua:10 are made to preserve the original ordering of some of the multi-icons status. I'd be happy to change it if anyone have different preferences :).

I think we can remove that. The order of statuses to icons retrieved by git.get_icons is Good Enough and easy to change following its normalisation.

Hmmm are you sure? Imo it looks quite weird having folders with the same combination of status being different, for example:

image

Here both marks and renderer have some files changed and some new files, but it looks like they have different git status.

Though tbh even with their icons sorted they'd still have different highlight. Maybe what we need is to sort the statuses?

@alex-courtis
Copy link
Member

Here both marks and renderer have some files changed and some new files, but it looks like they have different git status.

Though tbh even with their icons sorted they'd still have different highlight. Maybe what we need is to sort the statuses?

Whoops... I didn't notice that it was a maplike table. Options:

  1. revert 23f6276
  2. change it to an array-like with staged etc. as a table member and use utils.key_by

I like 1.

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please undo my bad idea: #1809 (comment)

@@ -60,19 +60,19 @@ local function warn_status(git_status)
end

local function get_icons_(node)
local git_statuses = explorer_common.get_git_status(node)
if git_statuses == nil then
local git_status = explorer_common.get_git_status(node)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually thinking we could use git_statuses everwhere as it is a much better variable name, similar to iconss.

However... looking through the whole codebase we are using git_status everywhere so I guess that change is outside of the scope of this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh sorry, I misunderstood. Perhaps I'll open a PR right after this one specifically for git_statuses?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're keen I would be most grateful! There are a lot of usages...

This reverts commit 23f6276.
@alex-courtis alex-courtis merged commit 29788cc into nvim-tree:master Dec 17, 2022
@alex-courtis
Copy link
Member

Many thanks for your contribution

@chomosuke chomosuke deleted the improve-dir-git branch December 17, 2022 08:29
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.

2 participants