Skip to content

Conversation

calmh
Copy link
Member

@calmh calmh commented Jan 15, 2024

Copy link
Member

@imsodin imsodin left a 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 :)

@calmh
Copy link
Member Author

calmh commented Jan 15, 2024

Frankly the whole ignore article should probably be rewritten to be much more user friendly but here we are

@calmh calmh added this pull request to the merge queue Jan 15, 2024
Merged via the queue into main with commit 6737b27 Jan 15, 2024
@calmh calmh deleted the ignoptim branch January 15, 2024 12:52
@@ -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::
Copy link
Member

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.

Copy link
Member

@acolomb acolomb left a 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
Copy link
Member

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.

Comment on lines +93 to +94
Negated patterns that can match items below the folder root will cause
Syncthing to traverse otherwise ignored directories. If the
Copy link
Member

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.

Comment on lines +105 to +107
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
Copy link
Member

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.

Comment on lines +108 to +109
``*``. As a special case, top-level rooted patterns (e.g. ``!/foo``) do
not cause this behaviour::
Copy link
Member

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?

Comment on lines +114 to +115
In this case, only the directory ``baz`` will be scanned, since
everything else is ignored by the ``*`` pattern.
Copy link
Member

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
Copy link
Member

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.

@acolomb
Copy link
Member

acolomb commented Jan 29, 2024

@calmh Are you going to follow up on this? See my previous review comments, specifically.

@syncthing syncthing locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants