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

org-ql-select fails with user error when buffer-or-files is a buffer-name #258

Open
ahmed-shariff opened this issue Oct 10, 2021 · 6 comments

Comments

@ahmed-shariff
Copy link

Continuation from #228

When org-ql-select is called with a buffer-name, it results with error

To replicate:

(with-temp-buffer
  (org-mode)
  (org-ql-select (list (buffer-name)) '(todo)))

Expected outcome:

  • work even if one of the elements in the buffer-or-files is a non file buffer.
@alphapapa
Copy link
Owner

Thanks, this is much easier for me to reason about.

In this case, I'm leaning toward requiring string arguments to be filenames. While Emacs's internal functions do tend to accept buffer names or buffer objects as arguments, those are functions that work specifically on buffers, not buffers or files as we do here. And org-ql-select is not an interactive command, so I see no reason for it to accept buffer names rather than buffer objects.

What do you think?

@ahmed-shariff
Copy link
Author

ahmed-shariff commented Oct 11, 2021

This actually stemmed from duplicate items being passed when using the interactive functions. For example, if a file name and also the buffer object of that file is passed in the interactive functions as two different values. The proposal in #260 would resolve them both to string values to allow removing duplicate values. Which results in the buffer objects being converted to string representations before being passed to the org-ql-select.

Is there any reason to not take buffer-names as a parameter?

On that note, should org-ql-select handle duplicate elements as well?

@alphapapa
Copy link
Owner

This actually stemmed from duplicate items being passed when using the interactive functions.

Okay, please, since we're starting over, let's get to the root problem first. But this issue isn't about duplicate items.

For example, if a file name and also the buffer object of that file is passed in the interactive functions as two different values.

Let's be specific: the interactive form of the command org-ql-search could pass buffers as buffers rather than as strings.

Is there any reason to not take buffer-names as a parameter?

Yes, because having both files and buffers passed as strings requires disambiguation, which, as we've seen, can be complicated. And since buffers don't have to be passed as strings, there seems no reason for them to be.

On that note, should org-ql-select handle duplicate elements as well?

I don't know what you mean. Anyway, please, let's stay focused in these discussions. If we start talking about related but separate issues, we'll end up having to start over again, again. If you want to discuss that, please open a separate issue.

This issue, #258, is about whether org-ql-select should accept buffer names as arguments. I think that, since it's not an interactive command, and since buffers can be passed as buffers, and since doing so would mean that a string argument must be a filename, it would simplify the code, and therefore be preferable to require buffers to be passed as buffers, not strings.

@ahmed-shariff
Copy link
Author

gotcha.

As for the disambiguating between buffers and file names, it's only one line (#259):

(get-buffer it)

I don't see any downside to supporting passing buffers-names as well.

@alphapapa
Copy link
Owner

The downside is making the code more complicated. If strings can be buffers or filenames, both cases must be accounted for, and that in various functions. The expansion/contraction of such arguments compounds that complexity. We've seen how awkward the code can be in the discussion about #228.

IME the case of needing to run org-ql-search on non-file-backed Org buffers, passing such buffers as arguments using the interactive form's completion, is very rare--perhaps so rare as to not justify this additional complexity. So I'd prefer to not support it in the first place. Then if some users say they need to do this regularly, we can consider supporting it, perhaps in a way that doesn't make code in 3+ functions much more complicated, as well as requiring extensive additional code in the tests to verify that it works correctly.

@ahmed-shariff
Copy link
Author

If my understanding of the current implementation is correct, that may make the code more complex? As I have described in #260, to account for the duplicate elements, I was resolving all values passed to buffers-files into strings, which would be either a file name or a buffer name if it's not a file backed buffer. If org-ql-select doesn't support non-file backed buffers, that would mean having to convert the buffer-names back to buffer objects before passing them to org-ql-select.

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 a pull request may close this issue.

2 participants