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

Fix: Correctly ignore files that start with a . (eg. .DS_Store) #406

Closed
wants to merge 0 commits into from

Conversation

jlwbr
Copy link

@jlwbr jlwbr commented Aug 27, 2024

This pr adresses 2 issues:

  1. [Bug]: Ignoring .DS_Store files doesn't seem to work #399: .DS_Store files where not ignored, this was caused by the pathlib library treating a file starting with a dot the same as a file without an extension. We now treat the whole filename as the extension in this case.
  2. When a file is already indexed in the library, but is excluded later, it will stay in the library (only new files are excluded). We now remove these files from the library when refreshing.

Closes #399

@CyanVoxel CyanVoxel added Type: Enhancement New feature or request Priority: Low Doesn't require immediate attention Status: Review Needed A review of this is needed TagStudio: Search The TagStudio search engine labels Aug 27, 2024
Copy link
Member

@CyanVoxel CyanVoxel left a comment

Choose a reason for hiding this comment

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

Thank you for this! I feel however this would be better as a toggle, perhaps to the left of the existing search options. I would also expect this to handle Windows' hidden file implementation, however I understand if that's out of the scope of what you're able to do.

Example:
image

Comment on lines 913 to 920
else:
entry_id = self.filename_to_entry_id_map.get(
f.relative_to(self.library_dir), None
)

if entry_id:
# A exclude file is in the library, let's remove it.
self.remove_entry(entry_id)
Copy link
Member

Choose a reason for hiding this comment

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

Not touching file entries that fall under the exclusion list is intended behavior. This would not only wipe entry data when playing with the exclusion list, but the user wouldn't even be notified of it.

Copy link
Author

Choose a reason for hiding this comment

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

I will remove this then!

@CyanVoxel CyanVoxel added Status: Changes Requested Changes are quested to this and removed Status: Review Needed A review of this is needed labels Aug 31, 2024
@eivl
Copy link
Collaborator

eivl commented Aug 31, 2024

Is this a to broad fix? seems overkill to exclude all files that are hidden under linux/macos/unix just to remove a macos .DS_Store file. wouldn't it make more sense to just ignore that file, and continue to extend the list of ignored files as they appear?

@jlwbr
Copy link
Author

jlwbr commented Aug 31, 2024

Is this a to broad fix? seems overkill to exclude all files that are hidden under linux/macos/unix just to remove a macos .DS_Store file. wouldn't it make more sense to just ignore that file, and continue to extend the list of ignored files as they appear?

That's not how this fix works, it just treats the name of the dot file the same as an extension of a normal file. (Eg excluding .json will exclude a.json and .json so a file with only an extension is treated the same as a file with an name and an extension.

@CyanVoxel
Copy link
Member

Is this a to broad fix? seems overkill to exclude all files that are hidden under linux/macos/unix just to remove a macos .DS_Store file. wouldn't it make more sense to just ignore that file, and continue to extend the list of ignored files as they appear?

That's not how this fix works, it just treats the name of the dot file the same as an extension of a normal file. (Eg excluding .json will exclude a.json and .json so a file with only an extension is treated the same as a file with an name and an extension.

I also presumed that this was hiding all files with a leading dot as Unix would, my mistake for not reading more closely.

I feel treating files with leading dots as toggleable hidden files instead of as extensions would make more sense, since that's how these files are designed to be treated. Plus if you had a file such as ".file.json", this wouldn't be registered as a proper ".json" file despite it being a valid file extension on a Unix hidden file.

@jlwbr
Copy link
Author

jlwbr commented Sep 1, 2024

Thank you for this! I feel however this would be better as a toggle, perhaps to the left of the existing search options. I would also expect this to handle Windows' hidden file implementation, however I understand if that's out of the scope of what you're able to do.

Example:

image

I will try this next week, I had the idea of changing up the system so that uses the same syntax as .gitignore this way, you can exclude whole files, all files with a particular extension, etc. But thought that that might be to big of a change for now and might also be to difficult to understand for some people without some solid ui around it.

@CyanVoxel
Copy link
Member

I will try this next week, I had the idea of changing up the system so that uses the same syntax as .gitignore this way, you can exclude whole files, all files with a particular extension, etc. But thought that that might be to big of a change for now and might also be to difficult to understand for some people without some solid ui around it.

There's actually an issue associated with this (#14) if you'd be interested in taking a stab at it

@CyanVoxel
Copy link
Member

@jlwbr Just wanted to touch base on this. A lot has changed in the codebase since this was last worked on, with the biggest being the merging of #332 which introduces a new library backend rendering this PR incompatible in its current state.

Would you still be interested in addressing #399?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Doesn't require immediate attention Status: Changes Requested Changes are quested to this TagStudio: Search The TagStudio search engine Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Ignoring .DS_Store files doesn't seem to work
3 participants