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

Taking hidden files parameter into account in windows #1684

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Catalyn45
Copy link
Contributor

fixes: #1683

On windows a file is considered hidden only if it has FILE_ATTRIBUTE_HIDDEN set. While this is the correct way to check if a file is hidden on windows, we should also take into account the hiddenfiles option so we can allow custom file patterns (ex. __pycache__, obj) as we can do on Linux.

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The code itself looks OK, but could you please do the following:

  • Update the documentation for the hidden option.
  • Check if the following linter comment is still relevant:

    lf/nav.go

    Lines 754 to 755 in ce12116

    //lint:ignore U1000 This function is not used on Windows
    func matchPattern(pattern, name, path string) bool {

Copy link
Collaborator

@joelim-work joelim-work left a comment

Choose a reason for hiding this comment

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

Looks good, thanks once again for the changes.

@gokcehan
Copy link
Owner

Thank you @Catalyn45 for the patch and @joelim-work for the review. I like the idea of enabling the use of hiddenfiles on Windows, but is it a good idea to hide dot files by default on Windows? Perhaps the default for hiddenfiles should be .* on unix and empty on Windows. What do you think?

@Catalyn45
Copy link
Contributor Author

I don't think we have other options that have a different default value depending on the os right? Maybe making this the only one will be a bit inconsistent and misleading.

Maybe a better alternative would be to put set hiddenfiles "" in the lf.ps1.example and lf.cmd.example.

Alternatively we can create some init_os_options at the end of options initialization.

@gokcehan
Copy link
Owner

@Catalyn45 Actually, we have shell and shellflag options with different values on Unix and Windows. Also, ifs option only works on Unix and not Windows. Unfortunately, I think the inconsistency is inherent in this case as there is no equivalent of hidden attributes on Unix. Nevertheless, I guess I don't have much of a strong opinion about this either way, so feel free to decide by yourself.

@joelim-work
Copy link
Collaborator

Ah, I did not consider the default value of hiddenfiles at all.

In that case, yes I agree with @gokcehan that this should be handled similarly to shell and shellflag (i.e. add definitions for gDefaultHiddenFiles in os.go and os_windows.go). The documentation also specifies the default values for each option, so that should be updated too.

@Catalyn45
Copy link
Contributor Author

Applied the suggestion, tell me if there is something else I should change.

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.

Hidden file patterns for windows
3 participants