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

Feature filter filename replacements #246

Conversation

anth-dinosaur
Copy link
Contributor

I wanted the ability to pull in relevant tasks to the current note (use case: a project overview note, for example). Inline dataview and templater expressions do not appear to process before the Todoist query, so I added some functionality to expand {{filename}} within the query filter. Spaces replaced with asterisks for compatibility.

I am not super familiar with typescript and this may be considered hacky. Future improvements would be to include a toggle in settings for this, maybe adding a map of other possible expansions, etc.

pass file name through to query parser
pass file name through to parser, and replace {{filename}} in filter query with file name (spaces removed to comply with Todoist, and remove .md extension)
Update to pass through file name, expand {{filename}} in query filter to file name, replacing spaces with wildcards for compatibility
Copy link
Owner

@jamiebrynes7 jamiebrynes7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great idea - just some thoughts on the correctness. I think having support for 'expansions' is generally a good idea so I might look to generalize this later. But for now this approach is a-okay.

src/query/injector.ts Outdated Show resolved Hide resolved
@anth-dinosaur
Copy link
Contributor Author

Glad you like the idea! I think it can be really powerful. Another idea I had for the future could be to add expansions for a custom frontmatter property you have defined in the note. Could allow for cleaner templates. Also, depending again on execution order, could allow for dynamic values to be passed in (if inline dataview or templater expression got processed first).

pendingquery has the ctx MarkdownPostProcessorContext which contains the source file name
- Had to use type any as I don't think the PendingQuery type is available in this context?
- Hopefully adding the .parsedObj to pendingQuery within parseQuery works here; tested fine when editing compiled js directly
- Filename seems to always be formatted with forward slashes, even on Windows, so only handled these in the regex
@anth-dinosaur
Copy link
Contributor Author

OK, this has been revised as discussed. This actually cleaned it up a bit as I didn't need to pass a new parameter around just for the filename, since the MarkdownPostProcessor context was already in the pendingQuery object. Added some comments on the commits with considerations.

Copy link
Owner

@jamiebrynes7 jamiebrynes7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a changelog entry to CHANGELOG.md in the root of the repository describing this feature?

@@ -19,20 +19,21 @@ export class ParsingError extends Error {
}
}

export function parseQuery(raw: string): Query {
export function parseQuery(pendingQuery: any): Query {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use PendingQuery as the type here? You'll need to export the type from injector.ts

if (!query.hasOwnProperty("name") || query.name === null) {
throw new ParsingError("Missing field 'name' in query");
}

if (!query.hasOwnProperty("filter") || query.filter === null) {
throw new ParsingError("Missing field 'filter' in query");
}

query.filter = query.filter.replace(/{{filename}}/g, pendingQuery.ctx.sourcePath.replace(/.*\//, '').replace(/\.md$/i, '').replace(/\s+/g, '*'));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This operation might be better done elsewhere. I.e. - parseObject validates that the object is the correct shape and returns a Query. Then we introduce another function that performs expansions on the Query

This enables a clearer delineation of responsibilities and makes it easier for us to test each method in isolation.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the series of regex replacements on the sourcePath, am I right in thinking this grabs the final file path part (i.e. - foo.md), drops the extension, and replaces whitespace characters with *?

Why do we want to do the last part?

@jamiebrynes7
Copy link
Owner

Hey @anth-dinosaur, I've pulled this change into #259, added some tests and tweaked the implementation. I've added you as a co-author on the commit - thanks for the contribution! 🎉

@anth-dinosaur
Copy link
Contributor Author

Hey @anth-dinosaur, I've pulled this change into #259, added some tests and tweaked the implementation. I've added you as a co-author on the commit - thanks for the contribution! 🎉

Thanks @jamiebrynes7!! Psyched to see this make it in. Apologies for having disappeared - I kept meaning to get back around to finishing the cleanup and it got buried. Thanks much for bringing it home.

@FrnklyN
Copy link

FrnklyN commented Jan 22, 2024

When is this going to happen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants