-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Allow the default filter run prefix to be specified to avoid unexpected deduplication #9595
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
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for tiddlywiki-previews ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Confirmed: Jermolene has already signed the Contributor License Agreement (see contributing.md) |
📊 Build Size Comparison:
|
| Branch | Size |
|---|---|
| Base (master) | 2451.3 KB |
| PR | 2453.6 KB |
Diff: ⬆️ Increase: +2.3 KB
✅ Change Note Status
All change notes are properly formatted and validated!
📝 $:/changenotes/5.4.0/#9595
Type: enhancement | Category: filters
Release: 5.4.0
Easier avoidance of deduplication in filters
🔗 #9595
👥 Contributors: Jermolene
📖 Change Note Guidelines
Change notes help track and communicate changes effectively. See the full documentation for details.
|
It would be nice to have a shortcut to toggle the same behaviour at the level of a filter expression. |
Yes, I think something like that would be very useful. Would a more general syntax such as that would obviate the need for this extension to the "subfilter" operator? Presumably the effects of the pragma would be inherited by a subsequent "subfilter" operator. I think that would work for #8702. I will investigate further. I note in passing that there is a precedent for using a suffix to control deduplication this approach in the "enlist" operator. |
I believe so and would in many cases be easier than needing to declare the filter expression as a variable and then go through the subfilter operator.
Agreed and I would not have any objections to also extending the |
It sounds like we're happy to merge this PR and then later develop the pragma approach. On that basis I'd like to go ahead with this PR to enable us to defer the policy and architecture work needed to get that right.
In a way that boat has sailed because we have so many existing operators with suffixes. Perhaps we need a genesis operator that allows any operator to be invoked, with the suffixes passed as parameters. |
Those usecases would indeed be better served by the proposed pragma syntax. The case for this PR is that there are plenty of situations where the subfilter/filter is already in a variable (or accessible by transclusion) and so the pragma syntax would be more unwieldy. Another point to consider is that the default filter run prefix is not inherited by nested subfilter/filter operators. I suspect that fixing that would be equivalent to implementing the pragma approach. |
Agreed.
The
An important distinction is that a pragma would apply to all filter runs in a filter expression, whereas filter run prefixes only apply to invididual filter runs (and potentially in the future their nested subfilter/filter operators). |
|
Widgets that accept filter expressions for For example: TiddlyWiki5/core/modules/widgets/genesis.js Lines 75 to 76 in e42ed68
Relevant widgets:
|
Co-authored-by: Saq Imtiaz <saq.imtiaz@gmail.com>
Interesting. I think it would be backwards compatible apart from situation where deduplication hides another bug. There might be a small usability issue in having to remember that filters behave subtly differently in different situations. I've committed the update in 5f03d1e but I am worried that it is a change that does not require users to explicitly opt-in to the alternate behaviour. It moves this PR a little closer to our abandoned idea of changing the default default filter run prefix. |
I think it helps that these are advanced widgets for which one always has to use deduplication. As long as we update the documentation for the widgets to clarify that for v5.4.0 these widget attributes are not deduplicated, I don't expect this to be problematic. The deduplication in these widget attributes is a common pitfall and came up yet again just last week. |
This reverts commit eb8f69d.
This PR extends the filter syntax to introduce "filter pragmas" identified by a double colon. They work similarly to their wikitext equivalent, influencing how the remainder of the filter is evaluated. The motivation is to make it possible to control whether deduplication is applied on a per-filter basis. For example:
Returns
1 2 2 1 4. Contrast to the result without the pragma which returns2 1 4:This PR also adds a parameter to the
subfilterandfilteroperators to specify the default filter run prefix to use when the filter is evaluated. This overrides any default set with the::defaultprefixpragma.We've long been considered ways to suppress deduplication during filter evaluation. An early experiment explored making deduplication a global setting. Our conclusion at the time was that the backwards compatibility issues were unacceptable. This PR is much narrower, requiring explicit opt-in to the new behaviour.
It's worth noting that the complexity of the regex used to parse operands is approaching warp speed:
(Diagram created with regexper.com)