obsidian-tasks-group / obsidian-tasks

Task management for the Obsidian knowledge base.
https://publish.obsidian.md/tasks/
MIT License
2.44k stars 228 forks source link

filename searches with parentheses fail to parse. #1852

Closed claremacrae closed 1 year ago

claremacrae commented 1 year ago

I am seeing an same issue with filenames containing parentheses:

The error message is:

> Error message is "do not understand query filter (filename)".

Error message is:

malformed boolean query -- Invalid token (check the documentation for guidelines)

I was really confused as to why a working Tasks query from months ago (granted, I didn't test it since February) suddenly didn't work anymore...

Originally posted by @TomWol in https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1500#issuecomment-1501216226

claremacrae commented 1 year ago

@tomwol I'm unclear which error message you are seeing.

As ever, the more info you can provide the better. Including the actual error message you receive. Thanks.

claremacrae commented 1 year ago

This is related to #1500 - but that is about filters with () inside Boolean logic.

This is different - it is a filter with no Boolean logic, that is matching BooleanField.canCreateFilterForLine() - or rather Field.canCreateFilterForLine(), even though there is no AND, OR, NOT, XOR in the filter line.

It then goes on to attempt to parse the line in `BooleanField.createFilterOrErrorMessage() - and it really is not a valid filter.

I have no idea why this fails for filename but not for path.

claremacrae commented 1 year ago

Here is the change I made to the tests, to reproduce the issue.

Index: tests/Query.test.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/Query.test.ts b/tests/Query.test.ts
--- a/tests/Query.test.ts   (revision 355025c99992b64f2320f789f399b8eb8e7a8a86)
+++ b/tests/Query.test.ts   (date 1681077444108)
@@ -17,6 +17,8 @@
 describe('Query parsing', () => {
     // In alphabetical order, please
     const filters = [
+        'filename does not include 2023-01-06 (Friday).md', // throws malformed boolean query -- Invalid token (check the documentation for guidelines)
+        'path does not include 2023-01-06 (Friday).md', // passes
         'created after 2021-12-27',
         'created before 2021-12-27',
         'created date is invalid',

(The individual Field implementations work fine, parsing the lines correctly - when you use the line for each field test)

claremacrae commented 1 year ago

@tomwol - this is not likely to be fixed any time soon, as the author of Boolean searches is currently doing a lot of work for Tasks on the CSS, and unlikely to have time to pick up the Boolean searching as well.

I recommend you use a workaround which is to do a regular expression search, and use . for the two parentheses - as close enough to give you the results you seek. On the assumption that you are unlikely to have files with that naming pattern and anything other than () in those positions.

/cc @esm7

TomWol commented 1 year ago

@TomWol I'm unclear which error message you are seeing.

As ever, the more info you can provide the better. Including the actual error message you receive. Thanks.

Of course. Thank you for creating a separate issue for it, I should have done this myself.

Working query

tasks
not done 
path does not include 2023-01-06 (Friday).md
path does not include Roadmap (Kanban).md
group by filename
group by heading
hide backlink

Queries that fail to parse

tasks
not done 
filename does not include 2023-01-06 (Friday).md
filename does not include Roadmap (Kanban).md
group by filename
group by heading
hide backlink

(The above fails with any number of filename filters, I included 2 for consistency between the 3 queries.)

tasks
not done 
(path does not include 2023-01-06 (Friday).md) AND (path does not include Roadmap (Kanban).md)
group by filename
group by heading
hide backlink
tasks
not done 
(filename does not include 2023-01-06 (Friday).md) AND (filename does not include Roadmap (Kanban).md)
group by filename
group by heading
hide backlink

Error message: Tasks query: malformed boolean query -- Invalid token (check the documentation for guidelines).

Operating System: macOS. Obsidian Version: 1.2.2 (installer: 1.1.16) Tasks Plugin Version: 3.1.0 I have tried it with all other plugins disabled and the error still occurs: confirmed.

claremacrae commented 1 year ago

Thanks for the examples.

Note to anyone working on this:

Please ignore the examples above that contain ) AND ( - they are the subject of a separate issue, #1500 - and not relevant to this one, which is intentionally for problems parsing a single filename search on one line.

esm7 commented 1 year ago

Although it is a different -- possibly simpler -- issue to solve, it seems like this direction for a fix should solve this one too.

esm7 commented 1 year ago

...having said that, the problem is very clear in the code:

const fieldCreators = [
    () => new StatusNameField(), // status.name is before status, to avoid ambiguity
    () => new StatusTypeField(), // status.type is before status, to avoid ambiguity
    () => new StatusField(),
    () => new RecurringField(),
    () => new PriorityField(),
    () => new HappensDateField(),
    () => new CreatedDateField(),
    () => new StartDateField(),
    () => new ScheduledDateField(),
    () => new DueDateField(),
    () => new DoneDateField(),
    () => new PathField(),
    () => new DescriptionField(),
    () => new TagsField(),
    () => new HeadingField(),
    () => new ExcludeSubItemsField(),
    () => new BooleanField(),
    () => new FilenameField(),
    () => new UrgencyField(),
    () => new RecurrenceField(),
];

The fact FilenameField comes after BooleanField causes the boolean field to consider the line first, look initially valid to the simple regex (which is issue #1500), but then fail when it goes down to actual parsing. BooleanField should be the last in that list. I made sure of that originally, but seems like I did not properly document the reasoning for that, and (expectedly) once new fields were added this convention broke.

  1. The problem is easily fixable by moving BooleanField to be last as it should have been.
  2. It's worth adding a test or an assertion in the code itself, and of course documentation, to validate it always remains last.
claremacrae commented 1 year ago

Thank you for spotting that @esm7. I do agree with your analysis.

I am unlikely to be able to get to this soon, due to the volume of other stuff in progress. so @TomWol I recommend using the workaround I suggested above for the time being.

claremacrae commented 1 year ago

This in-progress PR adds some more filters to the end of FilterParser so it will be easier to wait for that to be merged before dealing with this one:

esm7 commented 1 year ago

Any reason for them to really add them to the end? Although it's the "same cost" to fix for 3 fields or for 6, I see no reason to avoid worsening the problem further :shrug:

Anyway, I came here to say that I thought I'll have time today to work on this (and a bunch of other small Tasks things waiting for me), but previous obligations took longer than expected and this will need to wait to next week :(

claremacrae commented 1 year ago

Any reason for them to really add them to the end? Although it's the "same cost" to fix for 3 fields or for 6, I see no reason to avoid worsening the problem further 🤷

Anyway, I came here to say that I thought I'll have time today to work on this (and a bunch of other small Tasks things waiting for me), but previous obligations took longer than expected and this will need to wait to next week :(

At the time I think I thought that that PR was ready to go, and then I could quickly fix this on main...

aubreyz commented 12 months ago

OK I am pretty sure the problem I have is related to this thread, but the indication from April postings is that this is resolved. So I am posting here instead of starting a new issue.

I have thousands of pages where I am collating tasks which contain links to the current page. This has become vastly easier with the new code that allows dataview to be avoided. I have all of these queries as the below (to capture aliases as well).

```tasks
(description includes [[{{query.file.filenameWithoutExtension}}]]) OR (description includes [[{{query.file.filenameWithoutExtension}}|)
sort by description
```

This works great except when the current page name contains a bracket such as Joe Blogs (the older)

Then this fails with :

   Tasks query: malformed boolean query -- Invalid token (check the documentation for guidelines)

Is there any way to recode this to avoid this problem (or is this a different problem given the report that it is fixed)?

claremacrae commented 12 months ago

Hi @aubreyz, yes this one is specific to filename and fixed.

This is related to https://github.com/obsidian-tasks-group/obsidian-tasks/issues/1500 - but that is about filters with () inside Boolean logic.

The above is the one you want. Let's move over to there.