Skip to content

org-ql-search.el: Extend org-dblock-write:org-ql to indicate query scope#212

Closed
rekahsoft wants to merge 1 commit intoalphapapa:masterfrom
rekahsoft:master
Closed

org-ql-search.el: Extend org-dblock-write:org-ql to indicate query scope#212
rekahsoft wants to merge 1 commit intoalphapapa:masterfrom
rekahsoft:master

Conversation

@rekahsoft
Copy link
Copy Markdown

This is meant to resolve #151.

Namely, it adds a ":scope" parameter that can be used to indicate the scope of the query (the :from portion). It is inspired by how this is done with org clock tables.

I created this as a draft as it works functionally, but does not yet have tests. Happy to address any feedback.

@alphapapa
Copy link
Copy Markdown
Owner

Hi,

Thanks for your patience. This looks like a good start. I'll add some comments inline...

Comment thread org-ql-search.el
`buffer' the current buffer
`org-agenda-files' all agenda files
`org-directory' all org files
`(list-of-files ...)' a list of org files
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm glad you listed these choices explicitly. However, this list-of-files one needs clarification: is it an arbitrary Lisp list? Arbitrary Lisp code? A list of strings? A list of symbols which may evaluate to strings? etc.

Of course, it should probably only accept one string or a list of strings; any other types of value or a list of any other types of value should be rejected. However, we might consider accepting variables and lists of variables, which would make it easier for the user to refer to sets of files.

Either way, we should be very careful to avoid security issues with executing arbitrary code when the block is updated. Please see the tests related to the other block arguments for examples.

Comment thread org-ql-search.el
(-let* (((&plist :scope :query :columns :sort :ts-format :take) params)
(query (cl-etypecase query
(string (org-ql--query-string-to-sexp query))
(string (org-ql--plain-query query))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This change probably indicates that this patch is based on an outdated version of org-ql.

Comment thread org-ql-search.el
(org-ql--ask-unsafe-query query)
query)))
(columns (or columns '(heading todo (priority "P"))))
(scope (cond ((listp scope) scope)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

See first comment about arbitrary Lisp values.

Comment thread org-ql-search.el
(with-current-buffer (marker-buffer m)
(save-excursion
(goto-char m)
(org-store-link nil nil))))))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What's the reason for these changes? They seem unrelated to the PR's purpose.

Comment thread org-ql-search.el
(elements (org-ql-query :from scope
:where query
:select '(org-element-headline-parser (line-end-position))
:select 'element-with-markers
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why is this change made?

@maikol-solis
Copy link
Copy Markdown

maikol-solis commented Aug 23, 2021

This week started using orq-ql and this is a super useful feature. I started using org-ql-select and org-ql-query but the flexibility of using a dynamic block in any org-file to search remotely my to-dos will be impressive.

@alphapapa
Copy link
Copy Markdown
Owner

This should probably be merged with #239. @yantar92 FYI.

@alphapapa alphapapa added this to the 0.7 milestone Sep 22, 2021
@yantar92
Copy link
Copy Markdown
Contributor

yantar92 commented Oct 4, 2021

This should probably be merged with #239. @yantar92 FYI.

I have merged #239 with this taking into account your comments here. I guess we need to move further discussion to #239.

@rekahsoft
Copy link
Copy Markdown
Author

Thanks @yantar92 for taking over (been busy and haven't been able to come back to this). Closing this PR as its superseded by #239.

@rekahsoft rekahsoft closed this Dec 30, 2021
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.

[FEATURE] Add :file header argument to org-ql dynamic blocks

4 participants