kozec / syncthing-gtk

GTK3 & python based GUI for Syncthing
GNU General Public License v2.0
1.29k stars 140 forks source link

Add stignore support to nautilus #538

Closed TechupBusiness closed 3 years ago

TechupBusiness commented 4 years ago

I couldnt directly convert the go-code to python, so I ended up by writing the logic by myself. It should cover (all?) the cases mentioned in the documentation. But tests are welcome! :)

Feedback welcome!

TechupBusiness commented 4 years ago

@kozec here is another merge request :) Feedback and tests are welcome!

For example: how could I improve the stignore-loading? Right now it's only reloading the file when I kill nautilus (nautilus -q) or when cb_syncthing_folder_added is called.

Do you have an idea for icons that I could use for ignored files (the offline icon would be good but can be confusing sometimes)

AudriusButkevicius commented 4 years ago

I think the logic here does not match, as we stop processing ignores on the first hit.

kozec commented 4 years ago

For example: how could I improve the stignore-loading? Right now it's only reloading the file when I kill nautilus (nautilus -q) or when cb_syncthing_folder_added is called.

I would go with remembering mtime of .stignore file and checking it every time when folder is refreshed. That should work with almost no memory footprint unless someone has literally hundreds of synchronized folders.

Do you have an idea for icons that I could use for ignored files (the offline icon would be good but can be confusing sometimes)

How about not displaying any icon for those files? Or maybe transparent logo to communicate similar message as when IDE displays half-transparent icons for .gitignore-d files.

TechupBusiness commented 4 years ago

I think the logic here does not match, as we stop processing ignores on the first hit.

Thanks for your feedback @AudriusButkevicius ! Are you referring to this section?

# Check patterns for repo
for regex in self.ignore_patterns[repo]:
    # Performance advantage to check exclude before match (for the case is_ignored == True)
    if regex['exclude']:
        if is_ignored and regex['compiled'].match(path):
            is_ignored = False
    else:
        if not is_ignored and regex['compiled'].match(path):
            is_ignored = True

It's true that it continues to check further ignore conditions after hitting an ignore-exclude pattern. You mean the case that a file:

  1. it is ignored
  2. excluded from ignore (in a later pattern)
  3. syncthing stops here, there is no further processing to check further patterns (?)

To solve this issue I could simply add a break to the code above:

# Check patterns for repo
for regex in self.ignore_patterns[repo]:
    # Performance advantage to check exclude before match (for the case is_ignored == True)
    if regex['exclude']:
        if is_ignored and regex['compiled'].match(path):
            is_ignored = False
            break
    else:
        if not is_ignored and regex['compiled'].match(path):
            is_ignored = True

This way it stops further ignore processing when it hits a exclude match for a file. Then all following ignore patterns will be skipped.

TechupBusiness commented 4 years ago

For example: how could I improve the stignore-loading? Right now it's only reloading the file when I kill nautilus (nautilus -q) or when cb_syncthing_folder_added is called.

I would go with remembering mtime of .stignore file and checking it every time when folder is refreshed. That should work with almost no memory footprint unless someone has literally hundreds of synchronized folders.

Sounds good @kozec ! Would you go for another callback or is cb_syncthing_folder_added the right place? I noticed for example the events:

Do you have an idea for icons that I could use for ignored files (the offline icon would be good but can be confusing sometimes)

How about not displaying any icon for those files? Or maybe transparent logo to communicate similar message as when IDE displays half-transparent icons for .gitignore-d files.

Hm transparent emblem sounds good. Maybe we could use a green emblem with opacity < 1 for ignored directories that contains non-ignored files and remove any emblem on completely ignored files/folder (as implemented right now) or use this grey emblem with opacity < 1 (to indicate to the user that the file is within a syncthing folder)? Could you give me a hint how to add this emblem? Would save me some time, I don't understand the installation/setup completely yet :)

kozec commented 4 years ago

folder-scan-finished is probably better place, IIRC, folder-added fires only once, so you wouldn't have opportunity to reload .stignore at all.

Could you give me a hint how to add this emblem? Would save me some time, I don't understand the installation/setup completely yet :)

Just adding it to icons/ directory should be enough, setup.py already installs everything with "emblem-" prefix from there.

TechupBusiness commented 4 years ago

Just adding it to icons/ directory should be enough, setup.py already installs everything with "emblem-" prefix from there.

Thanks! For development (with apt installation) I needed to copy the icon with proper naming scheme to /usr/share/icons/hicolor/64x64/emblems (found via dpkg-query -L syncthing-gtk).

And then run: sudo update-icon-caches /usr/share/icons/* to refresh the cache manually.

I made the "ignored-parent-path" 50% transparent. Do you think @kozec its enough for clearly seeing the difference in the folder structure? image The second folder is ignored but it contains non-ignored content (on any deeper level).

TechupBusiness commented 4 years ago

I think the logic here does not match, as we stop processing ignores on the first hit.

@AudriusButkevicius could you please give some more details of how syncthing is doing things different? Did you mean https://github.com/syncthing/syncthing-gtk/pull/538#issuecomment-548151446 ? Thanks!

kozec commented 4 years ago

Do you think @kozec its enough for clearly seeing the difference in the folder structure?

Yes, I can see difference clearly.

TechupBusiness commented 4 years ago

Found out that stignore behaves unfortunately different to gitignore... not really intuitive... Edit: just opened a discussion https://forum.syncthing.net/t/gitignore-logic-for-stignore-v2/14064

st-review commented 4 years ago

:robot: beep boop

I'm going to close this pull request as it has been idle for more than 90 days.

This is not a rejection, merely a removal from the list of active pull requests that is periodically reviewed by humans. The pull request can be reopened when there is new activity, and merged once any remaining issues are resolved.

TechupBusiness commented 4 years ago

I'm still using it and it works like a charm. Would you merge it @kozec ? It's not perfect but better than without :)

kozec commented 4 years ago

Sorry about that bot, he is really stealthy and doesn't generate email notifications.

I'll do brief test just to check it doesn't explode with Nemo and merge it.