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

feat(ui): hide hidden files irrespective of hide_gitignored option #946

Merged
merged 2 commits into from
Jun 9, 2023
Merged

feat(ui): hide hidden files irrespective of hide_gitignored option #946

merged 2 commits into from
Jun 9, 2023

Conversation

pysan3
Copy link
Collaborator

@pysan3 pysan3 commented May 24, 2023

closes #929

Note

show_anyways is only used in this context (there is no other reference to show_anyways other than my fix), so deleted in line 200.

Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

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

I don't understand how this would fix the problem described in the bug. I expected the fix to be that the choice of whether to hide something be fixed in renderer.lua --> remove_filtered only so that some choices take precedence over others.

@pysan3
Copy link
Collaborator Author

pysan3 commented May 24, 2023

I'm sorry @cseickel

Forgot to turn it into a draft.

I was intending to first get a reply over on the issue and then start working on it.

@cseickel
Copy link
Contributor

I was intending to first get a reply over on the issue and then start working on it.

Ah, well yes, I do think this is something that should be fixed!

@pysan3 pysan3 marked this pull request as draft May 24, 2023 13:07
@pysan3
Copy link
Collaborator Author

pysan3 commented May 24, 2023

Hey @cseickel I've got a question,

With further exploration, I think the easiest way would be to skip updating item.filtered_by.gitignored by short circuiting everything here.

local function finalize(all_results)
local show_anyway = state.filtered_items and state.filtered_items.hide_gitignored == false
log.trace("IGNORED: Comparing results to mark items as ignored, show_anyway:", show_anyway)
local ignored, not_ignored = 0, 0
for _, item in ipairs(items) do
if M.is_ignored(all_results, item.path, item.type) then
item.filtered_by = item.filtered_by or {}
item.filtered_by.gitignored = true
item.filtered_by.show_anyway = show_anyway
ignored = ignored + 1
else
not_ignored = not_ignored + 1
end
end
log.trace("IGNORED: mark_ignored is complete, ignored:", ignored, ", not ignored:", not_ignored)
log.trace("================================================================================")
end

image

I at first thought this would affect other places since it does not flag filtered_by.gitignored, but in fact this is what is done for other hide_* configs so I think it is rather a feature? config.hide_dotfiles == false makes it never update filtered_by.hidden in the first place.

if f.hide_dotfiles and string.sub(name, 1, 1) == "." then
item.filtered_by = item.filtered_by or {}
item.filtered_by.dotfiles = true
end
if f.hide_hidden and utils.is_hidden(path) then
item.filtered_by = item.filtered_by or {}
item.filtered_by.hidden = true
end

Theoretically this short circuit will be a breaking change, but I would say that updating filtered_by.gitignored when config.hide_gitignored == false was unnecessary in the first place.

Would you agree with this fix @cseickel ?

Another option would be to add if-elses here as below. Which however I feel is a bit inconsistent.

image

Which one would you prefer?

@pysan3
Copy link
Collaborator Author

pysan3 commented Jun 6, 2023

@cseickel Any updates?

@cseickel
Copy link
Contributor

cseickel commented Jun 6, 2023

The part you have to consider is that even when a gitignored item is visible, we still need to be able to mark it as ignored in the name and git_status components so it shows with teh right font and with the correct status symbol.

Your second proposal is the correct strategy. I don't know for sure if the logic is correct or not (short on time), but it is definitely the right place to handle this.

@pysan3 pysan3 marked this pull request as ready for review June 9, 2023 07:42
@pysan3
Copy link
Collaborator Author

pysan3 commented Jun 9, 2023

@cseickel Thanks for the response.

Your second proposal is the correct strategy

I've updated the code and is ready to be merged :)

Copy link
Contributor

@cseickel cseickel left a comment

Choose a reason for hiding this comment

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

You're on a roll @pysan3!

@cseickel cseickel merged commit 389eef7 into nvim-neo-tree:main Jun 9, 2023
3 checks passed
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.

None yet

2 participants