-
Notifications
You must be signed in to change notification settings - Fork 567
users/ignoring: Update notes on negated ignore patterns and scanning/watching #854
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite nice and clear to me, but that's not very significant as I knew how it works before reading it already - we'll see how users take it :)
Frankly the whole ignore article should probably be rewritten to be much more user friendly but here we are |
@@ -98,26 +124,15 @@ The ``.stignore`` file contains a list of file or path patterns. The | |||
they are preventing directory deletion. This prefix should be used by any OS | |||
generated files which you are happy to be removed. | |||
|
|||
.. note:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd leave this one outside of the bullet list, as it applies to the last two points equally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@calmh Sorry for being late. I do think that the text definitely needs improvments to make it easier to grasp. Some suggestions follow, but I didn't try to rewrite it completely. Also because I'm not completely sure I did understand it myself.
|
||
Prefixes can be specified in any order (e.g. ``(?d)(?i)``), but cannot | ||
be combined in a single pair of parentheses like :strike:`(?di)`. | ||
|
||
- A line beginning with ``//`` is a comment and has no effect. The same double |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be shuffled way up higher IMO, probably close to the /rooted_ignore_pattern
bullet point. Then the closing note about prefix order stands correctly after the list, but still directly related to the prefixes.
Negated patterns that can match items below the folder root will cause | ||
Syncthing to traverse otherwise ignored directories. If the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This first sentence requires serious mental capacity to parse, even for me as an advanced user. Better to split it into smaller statements:
"Syncthing will sometimes look at the files even inside ignored directories. Some file that matches any negated pattern (thus, should be included) could very well reside somewhere below them. This happens only for directories matching an ignore pattern after the first negated pattern."
Then the example, with speaking directory names that directly show what happens:
/1-not_looked_at
/2-also_completely_disregarded
!3-filename_to_sync_anyway
!4-foo
/5-could_have_foo_below_so_scan_it
*
Not that this is the ideal solution, but hopefully easier to grasp. So I hope you don't mind me just sketching the concept without following through.
The directories ``foo`` and ``bar`` will be entirely ignored. However any | ||
other directories present must be scanned entirely to find any items | ||
named `baz`, despite the fact that they will be ignored due to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also a difficult sentence, should be split up at the comma and refactored. Speaking names in the example will help to reference easily. Include a number to avoid repeating long names here.
``*``. As a special case, top-level rooted patterns (e.g. ``!/foo``) do | ||
not cause this behaviour:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far "this behavior" (U.S. English seems to be used mostly in the docs) is not very well understood, which makes it hard to see what exactly top-level rooted patterns really do. They're obviously negated, but don't cause something which essentially meant not doing something. So do they do it or don't they? 🤯
And "top-level rooted patterns" is a bit too terse for parsing correctly IMO, even with the example. The example is also negated btw. so does the remark apply to all top-level rooted negated patterns or what?
In this case, only the directory ``baz`` will be scanned, since | ||
everything else is ignored by the ``*`` pattern. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite having seen (but not thoroughly checked) the related PR, this special case suprised me. I.e. could be better explained.
!baz | ||
* | ||
|
||
The directories ``foo`` and ``bar`` will be entirely ignored. However any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A note about the version where this optimized behavior was introduced would help. Yes, we do have the docs versioning, but an unsuspecting user might not come up with the idea that the handling was ever different.
@calmh Are you going to follow up on this? See my previous review comments, specifically. |
Ref syncthing/syncthing#9340