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

Behavior change in handling \ at the very end of tasks queries [in Live Preview, due to Obsidian unintended change] #3137

Closed
5 of 13 tasks
blakeNaccarato opened this issue Oct 21, 2024 · 17 comments · Fixed by #3166
Labels
display: live preview Issues referring to Obsidian's Live Preview mode scope: query logic Boolean combinations of filters - and, or, not type: bug Something isn't working type: no action needed Conclusion of the discussion was that no changes to Tasks are required

Comments

@blakeNaccarato
Copy link

blakeNaccarato commented Oct 21, 2024

Please check that this issue hasn't been reported before.

  • I searched previous Bug Reports didn't find any similar reports.

Steps to reproduce

I recently updated Obsidian Tasks after few months of not updating, and lots of my queries have started returning nearly every task in my vault. Queries which had a final \, e.g. in the "leaky" query below, would slow down Obsidian significantly and list all tasks in the vault. (The reason I often have trailing \ in queries is that I tend to mix/match query snippets and keeping the trailing \ is robust to changes in that query, e.g. moving that "clause" up or down, or elsewhere).

I found that removing the \ at the very end of the query restored functionality to what I expected from before updating, properly filtering tasks.

I would say tentatively that this apparent "regression" has cropped up in one of the updates in the past ~six months. If there's an easy methodology for me to quickly "bisect" Tasks plugin versions to narrow down the exact release this happened on, or Obsidian Sync logs I'm unaware of, please let me know. My Sync history only shows in the manifest my recent update to 7.10.2, then updating to 7.11.1, so I can't exactly recall which version I updated from when this regression crept in.

Simple reproduction

  1. Create a new vault
  2. Enable community plugins and install Tasks
  3. Create two files, one.md and two.md with the following contents
  4. Switch to Live Preview (tested in Obsidian 1.7.4 - this line added by @claremacrae)
  5. Note the difference between the "leaky" and "correct" query in two.md. The only difference is the trailing slash. The leaky query "incorrectly" sees the task in one.md.
  • (Not yet done by myself) To find the version in which the regression appeared, install progressively older versions of the plugin until slash handling in the "leaky" query is handled the same as in the new "correct" query
one.md
# One

## Task

- [ ] one
two.md
# Two

## Task

- [ ] two

## Leaky query (trailing slash)

```tasks
( path includes {{ query.file.path }} )\
```

## Correct query (no trailing slash)

```tasks
( path includes {{ query.file.path }} )
```

Expected Behavior

Queries with trailing \ should still function properly.

Current behaviour

Queries with trailing \ fetch everything in the vault instead of filtering as expected.

Which Operating Systems are you using?

  • Android
  • iPhone/iPad
  • Linux
  • macOS
  • Windows

Obsidian Version

1.7.4

Tasks Plugin Version

7.11.1

Checks

  • I have tried it with all other plugins disabled and the error still occurs

Possible solution

  • Verify reproduction of this behavior
  • Determine whether this is intended behavior. If so, consider communicating it in docs. If not, proceed...
  • Determine the version in which the behavior change was introduced and find the PR that changed it
  • Consider implementing a "fix" for this "regression" if deemed as such
  • Add a test to the test suite that checks the case of "backslash is the final character in the query before closing backticks"
@blakeNaccarato blakeNaccarato added the type: bug Something isn't working label Oct 21, 2024
@claremacrae
Copy link
Collaborator

Hi Blake,

Thanks for using Tasks.

Cannot reproduce the problem

With the exactly query you gave, I cannot reproduce the problem.

image

The change in behaviour

I recently updated Obsidian Tasks after few months of not updating

I maintain a special page for that situation:

https://publish.obsidian.md/tasks/What+is+New/Breaking+Changes

By coincidence, the feature that changed that behaviour celebrated its first birthday today!

Tasks 5.0.0 (21 October 2023)

The meaning of final backslash () characters on query lines

Reading that section of the docs will enable you to update your searches to get the original behaviour

@claremacrae claremacrae added the scope: query logic Boolean combinations of filters - and, or, not label Oct 21, 2024
@claremacrae
Copy link
Collaborator

PS I recommend adding the explain instruction at the start of the query, whenever you have unexpected search results - or to verify your searches are doing what you expect.

image

@claremacrae claremacrae added the type: no action needed Conclusion of the discussion was that no changes to Tasks are required label Oct 24, 2024
@claremacrae
Copy link
Collaborator

I’ll assume that no news is good news, and close this

@claremacrae claremacrae closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2024
@blakeNaccarato
Copy link
Author

blakeNaccarato commented Oct 30, 2024

Sorry I've been out of town the past week.

After revisiting my minimal reproducible example, I've found that the buggy behavior I describe is only happening in the Live Preview mode, but not in the Reading Mode. Here we see that Reading Mode is correct, even with the trailing slash. See the screenshot below (with hasty red PowerPoint annotations are mine) where I present three views of the same note. So query parsing in Live Preview mode somehow differs from Reading Mode for the same source query!

Here's a zipped up test vault showing this behavior, produced from my Windows machine just now.

Thanks for the pointer to use explain and no global query. They seem to be special queries that don't require parentheses wrapping and line continuation as is the case with complex AND/OR/AND NOT type tasks queries.

image

@claremacrae claremacrae reopened this Oct 30, 2024
@claremacrae
Copy link
Collaborator

Sorry I've been out of town the past week.

Thanks for replying. I've re-opened the ticket.

Why I encouraged you to use the explain instruction

Thanks for the pointer to use explain and no global query.

I didn't actually say to use no global query/ignore global query...

It was included in my query screenshot to prevent my own personal global query from being included in my results screenshot, which would have leaked personal information.

They seem to be special queries that don't require parentheses wrapping and line continuation as is the case with complex AND/OR/AND NOT type tasks queries.

No, what I actually said was:

PS I recommend adding the explain instruction at the start of the query, whenever you have unexpected search results - or to verify your searches are doing what you expect.

And indeed, if you had added that line at the start of your query, it would have shown you what the difference in behaviour was!

Sample vault

After revisiting my minimal reproducible example, I've found that the buggy behavior I describe is only happening in the Live Preview mode, but not in the Reading Mode. Here we see that Reading Mode is correct, even with the trailing slash. See the screenshot below (with hasty red PowerPoint annotations are mine) where I present three views of the same note. So query parsing in Live Preview mode somehow differs from Reading Mode for the same source query!

Here's a zipped up test vault showing this behavior, produced from my Windows machine just now.

Thank you - that is really helpful.

When I open your vault, and add explain at the start of the query, this is what I see:

image

Specifically, It shows the difference in query interpretation between the two modes.

The reason for putting explain at the start of the query is for safety - it ensures that it is not affected by any of the later elements, such as continuation characters.

What is happening

Someone reported in Discord recently that there is an inconsistency between how Obsidian parses code blocks between Reading Mode and Live Preview. One has an extra space at the end - I forget which, and it looks like they didn't log it on the forum.

We can test the behaviour by adding a newline at the end of the two query blocks.

Now we get the same results from both:

image

What next?

The right answer is not to have a trailing backslash at the end of the query.

@claremacrae
Copy link
Collaborator

The right answer is not to have a trailing backslash at the end of the query.

And the difference in behaviour between Live Preview and Reading mode is an Obsidian bug, and outside of Tasks' control.

@blakeNaccarato
Copy link
Author

blakeNaccarato commented Oct 31, 2024

Thanks for the detailed reply! I think the primary thing worth communicating here, perhaps in an appendix/callout/admonition added to https://publish.obsidian.md/tasks/Queries/Line+Continuations, is that this behavior change has emerged recently (probably in the stable release of Obsidian 1.7.4 this month) and can manifest as very slow queries all over the vault trying to collect all tasks in the vault.

I've drafted a quick PR (#3163) adding such a mention to the line continuations docs, if you deem it reasonable to communicate that. I think perhaps the stable/public release of Obsidian 1.7.4 is the culprit, which includes the Catalyst/preview changes from Obsidian 1.7.2, judging by the changelog entry Live preview now only escapes special characters (not letters and numbers).. If the preferred route is to leave it undocumented in hopes of Obsidian releasing a fix soon, then feel free to close the PR unmerged, though it may lead to noisy issues/discussions if more line continuation users get onto latest Obsidian.

I haven't verified this by installing different isolated versions of Obsidian and seeing when the behavior emerges, but I do believe it to be a recent behavior change since it wasn't happening to me until recently.


Probably my language was imprecise when mentioning the explain/no global query stuff. With your guidance, I did experiment with explain, but also experimented with no global query because I am less familiar with the project's architecture and thought the global query may be participating in this strange behavior. The varying explanation output across preview/reading mode led me to screenshot it, but ultimately I omitted explain in the minimal example in case its presence was somehow affecting the result (again through unfamiliarity with the implementation of explain). Anyways, thanks again!

blakeNaccarato added a commit to blakeNaccarato/obsidian-tasks that referenced this issue Oct 31, 2024
…continuations

See obsidian-tasks-group#3137. From that discussion, added an appendix entry to this indicating the Live Preview behavior change since Obsidian 1.7.2 that leads to malformed queries *only in Live Preview mode*. This upstream Obsidian bug affects Obsidian Tasks code blocks and can manifest as surprisingly slow queries running to return all tasks in the vault.

The culprit may be  [Obsidian 1.7.2](https://obsidian.md/changelog/2024-09-19-desktop-v1.7.2/), judging by the changelog entry `Live preview now only escapes special characters (not letters and numbers).`.

See [my comment](obsidian-tasks-group#3137 (comment)) and [this comment](obsidian-tasks-group#3137 (comment)) for more detail.

- [ ] Before merging this documentation change, verify that Obsidian v1.7.2 really is the culprit here
@claremacrae
Copy link
Collaborator

claremacrae commented Oct 31, 2024

Someone today shared the link to the Forum report of the extra new line:

https://forum.obsidian.md/t/bug-report-markdowncodeblockprocessor-adds-extra-newline-when-in-reading-mode/90197

Blake, the thing I am missing in all of this is why you had query ending with a trailing slash at the end of a path query?

Edit: or at the end of a boolean expression like that? What purpose were you expecting it serve?

I would also want to know whether you previously using a pre-Tasks-7.0.0 release?

Thanks for the PR. I don't know enough yet to make a decision on what to do, if anything, with this ticket - it will impart include understanding the curious \ you had in your query, which was never (meant to be) valid search Tasks syntax.

@claremacrae
Copy link
Collaborator

Note: I've made some edits to the comment above...

@blakeNaccarato
Copy link
Author

blakeNaccarato commented Oct 31, 2024

EDIT: See my follow-up comment below, which may make the rest of this long-winded comment moot. If only I was out of town for another week maybe I wouldn't have caused us both to spend time chasing a problem that will be fixed soon anyways 😅.


Some of my queries for "dashboard"-like overview documents use boolean combinations, use only parentheses as a delimiter, don't use quotes, and use line continuations for readability/refactorability in complex logic. When refactoring queries like in the example I give below, sometimes the final line ends up moving, not being the final line anymore, so keeping the trailing slash there makes it consistent and easy to move without breaking the query. Not being able to include a final trailing slash creates a usability annoyance similar to how the lack of a trailing comma in a final JSON element means it will break if you move that element without adding a comma to it.

I'm pretty sure I have been using at least 7.0 at for the past six months, but I wrote queries like the example below during v6 and haven't modified them much. And I think we've found that my recent update from ~7.0 to the latest Tasks is not the root cause of the behavior change, rather it's probably the latest version of Obsidian changing Live Preview (will need to be confirmed before decision on the PR).

Maybe the upstream bug be fixed quickly in Obsidian proper and this issue topic is all that's needed to signpost the danger to other users of Obsidian Tasks. In any case, it's fine to decline the docs contribution if this topic suffices to document it!

More detail

Details

I guess the minimum example doesn't really illustrate the "why" of wanting a final trailing slash at all. So below is an example of the kinds of "dashboard"-like tasks documents I use to monitor tasks of a given kind throughout a vault. While the tasks queries below include a lot of boilerplate/repetition, they're fairly easy to manage with a little automation. Use of the explicit ANDs and ORs, and splitting queries across lines make it much easier for one query to exclude results already handled by another query by reuse and negation of a given "sub-query".

Until the recent behavior change of a trailing slash in a given code block breaking Live Preview query parsing, all of my more "complex" queries like this had final trailing slashes like AND ( status.type IS IN_PROGRESS )\. In case that line got moved up in the query and was no longer the final line, the query wouldn't break.

I know that the "philosophy" of the tasks plugin is to instead set up sorting/filtering rules within one master query, and use task priorities and such to simplify queries, avoiding a lot of the "boilerplate" you see below. But I just wasn't able to craft the queries I felt I needed using simple query language. Possibly the example below is a "tasks smell", and just is the "wrong" way to go about task dashboarding, but I think the "final trailing slash" issue may crop up even in simpler queries.

Anyways, I've scrubbed my vault of all final trailing slashes to sidestep the current Live Preview issue. It's not really a big deal, but I imagine that anyone making heavy use of line continuations and boolean combinations to craft queries might be bit by the final trailing slash bug and end up with a query that vomits all the tasks in the vault in Live Preview mode, resulting in Obsidian stalling/hanging.

Contents of to-do/grad/_grad.md (a "folder note")


Grad

Due

(\
    ( path includes {{ query.file.path }} )\
    OR ( path INCLUDES _notes/ )\
    OR ( path INCLUDES coding/ )\
    OR ( path INCLUDES grad/ )\
    OR ( path INCLUDES _reviews/ )\
    OR ( tag INCLUDES grad )\
)\
AND NOT (\
    ( path INCLUDES _&to-do/jobs/ )\
    OR ( path INCLUDES _&to-do/mind/ )\
    OR ( path INCLUDES _&to-do/personal/ )\
    OR ( path INCLUDES _&to-do/watch/ )\
)\
AND NOT ( done )\
AND ( HAS due date )\

Scheduled

(\
    ( path includes {{ query.file.path }} )\
    OR ( path INCLUDES _notes/ )\
    OR ( path INCLUDES coding/ )\
    OR ( path INCLUDES grad/ )\
    OR ( path INCLUDES _reviews/ )\
    OR ( tag INCLUDES grad )\
)\
AND NOT (\
    ( path INCLUDES _&to-do/jobs/ )\
    OR ( path INCLUDES _&to-do/mind/ )\
    OR ( path INCLUDES _&to-do/personal/ )\
    OR ( path INCLUDES _&to-do/watch/ )\
)\
AND NOT (\
    ( done )\
    OR ( HAS due date )\
)\
AND ( HAS scheduled date )\

Doing

(\
    ( path includes {{ query.file.path }} )\
    OR ( path INCLUDES _notes/ )\
    OR ( path INCLUDES coding/ )\
    OR ( path INCLUDES grad/ )\
    OR ( path INCLUDES _reviews/ )\
    OR ( tag INCLUDES grad )\
)\
AND NOT (\
    ( path INCLUDES _&to-do/jobs/ )\
    OR ( path INCLUDES _&to-do/mind/ )\
    OR ( path INCLUDES _&to-do/personal/ )\
    OR ( path INCLUDES _&to-do/watch/ )\
)\
AND NOT (\
    ( done )\
    OR ( HAS due date )\
    OR ( HAS scheduled date )\
)\
AND ( status.type IS IN_PROGRESS )\

Start

(\
    ( path includes {{ query.file.path }} )\
    OR ( path INCLUDES _notes/ )\
    OR ( path INCLUDES coding/ )\
    OR ( path INCLUDES grad/ )\
    OR ( path INCLUDES _reviews/ )\
    OR ( tag INCLUDES grad )\
)\
AND NOT (\
    ( path INCLUDES _&to-do/jobs/ )\
    OR ( path INCLUDES _&to-do/mind/ )\
    OR ( path INCLUDES _&to-do/personal/ )\
    OR ( path INCLUDES _&to-do/watch/ )\
)\
AND NOT (\
    ( done )\
    OR ( HAS due date )\
    OR ( HAS scheduled date )\
    OR ( status.type IS IN_PROGRESS )\
)\
AND ( HAS start date )\

@blakeNaccarato
Copy link
Author

blakeNaccarato commented Nov 1, 2024

A recent comment suggests they're working on fixing it in 1.7.5, so perhaps the documentation PR might cause some unnecessary churn! My initial concern was whether or not this would be fixed upstream promptly and cause noise in Obsidian Tasks in the meantime. Anyways, thanks for your help in identifying the root cause while showing me some new tasks tricks along the way!

Feel free to re-close this issue and/or the PR as you see fit.

@claremacrae
Copy link
Collaborator

Thanks for both your messages, which I will reply to in full when I have some time.

I am actually intending to make Tasks adjust the query input to prevent the problem case from happening.

As well as working around the current Obsidian issue, this issue has shown it will be necessary to be done in advance of #2459, to prevent confusing behaviours if users pass in queries like that currently created in Live Preview...

@claremacrae claremacrae added the display: live preview Issues referring to Obsidian's Live Preview mode label Nov 1, 2024
@claremacrae claremacrae changed the title Behavior change in handling \ at the very end of tasks queries Behavior change in handling \ at the very end of tasks queries [in Live Preview, due to Obsidian bug] Nov 1, 2024
@claremacrae claremacrae changed the title Behavior change in handling \ at the very end of tasks queries [in Live Preview, due to Obsidian bug] Behavior change in handling \ at the very end of tasks queries [in Live Preview, due to Obsidian unintended change] Nov 1, 2024
@claremacrae
Copy link
Collaborator

claremacrae commented Nov 1, 2024

My initial plan was simply to make the Query class append a \n to the end of the query text, if the last character was not already a newline.

However, upon investigation, the logic error is in here:

// Decide what to do with the next line:
if (endsWith2Slashes(inputLine)) {
continuePreviousLine = false;
} else {
continuePreviousLine = endsWith1Slash(inputLine);
}
if (!continuePreviousLine) {
if (currentStatementProcessed.trim() !== '') {
instructions.push(new Statement(currentStatementRaw, currentStatementProcessed));
}
currentStatementRaw = '';
currentStatementProcessed = '';
}
}
return instructions;
}

If:

  1. there is no newline character at the end of the query string...
  2. ... and the last line of the query string ends in \

Then:

  • the function returns with the last line of the query still stored in currentStatementProcessed, instead of having been appended to instructions.

The simplest fix is probably to adjust the if condition on line 88 to say:

if (!continuePreviousLine || weAreOnTheLastLineOfTheFile) {

@claremacrae
Copy link
Collaborator

I've edited the reproduction steps in the first post above to add the critical information that it needs to be viewed in Live Preview, and it likely matters that Obsidian 1.7.4 is used...

claremacrae added a commit that referenced this issue Nov 2, 2024
…n-last-line

fix: '\' at end of last line in query no longer discards the instruction

Fixes #3137
@claremacrae
Copy link
Collaborator

@blakeNaccarato Many thanks for discovering and logging this.

I have fixed the underlying cause in the Tasks code in PR #3166.

For all the hours spent on understanding and discussing the problem, it turned out there was simple (and acceptable) workaround in code to prevent the problem.

I will release this when I have time - perhaps later today.

And I do still plan to reply to some of the discussion above, where I feel clarification is worthwhile.

@claremacrae
Copy link
Collaborator

Released in 7.12.3

@blakeNaccarato
Copy link
Author

Great! Glad I could help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display: live preview Issues referring to Obsidian's Live Preview mode scope: query logic Boolean combinations of filters - and, or, not type: bug Something isn't working type: no action needed Conclusion of the discussion was that no changes to Tasks are required
Projects
None yet
2 participants