Allow multiple values in `org-ql-view-buffers-files'

[ahmed-shariff]

This is a revisit to the #228, apologize for not being able to get back to this sooner. That PR had started trying to fix the init-value of the completing-read (#227) and wound up doing alot of other things.

The goal of the PR is as follows:

• From the org-ql-view allow setting the org-ql-view-buffers-files using completing-read-multiple

This entails the following fixes:

• Ensure the values from expanding completing-read-multiple are correct and don't have duplicates
• Ensure the contraction of the values of org-ql-view-buffers-files are handled correctly
• The org-ql-view--complete-buffers-files uses the correct contracted form, also respect any functions set it's instead.

What I have here also had an interesting side-effect on the link-safety tests:

• Incorrect values to buffers-or-files would cause the org-ql-view--expand-buffers-files signal an error at https://github.com/alphapapa/org-ql/blob/5f70636556bffca92d8ef8297ba3002a4ab5b52d/org-ql-view.el#L634 as opposed to to being signaled at https://github.com/alphapapa/org-ql/blob/5f70636556bffca92d8ef8297ba3002a4ab5b52d/org-ql-view.el#L639 I could have caught the error in the former and let the latter signal the error, but that didn't feel right to me

[alphapapa]

Hi again,

After skimming through #228 again, I feel like quoting my last comment:

I realize that you've put a lot of work into this patch, and that you've been very patient with my requested changes, so I don't want to discard your work. Would you be willing to consider starting this process over, defining the problems more explicitly in an issue, then addressing them in one or more new PRs, which could reuse the code from this one? I think this is a case where some degree of test-driven development would be helpful--at least, for me, as I try to understand the problems we're trying to solve and exactly how we're trying to do it.

For example, I'd like to first have the problems clearly described in one or more issues, then have a PR for each one, each of which should start by adding a failing test that should be fixed by later commits. I'm trying to develop this package in a more organized way to ensure that it remains reliable and robust, but that does sometimes mean more overhead for contributions. As well, since I work on this in spare time, it sometimes takes a while to review and integrate patches.

In other words, this still feels like too much to review, and we still lack carefully written problem statements which can be clearly and simply addressed.

I do appreciate your work on this PR, and I especially noticed how much you put into the test cases. But, again, with this kind of issue--especially one that could present a security vulnerability--we need to be methodical.

In the end, this may be an issue that I need to solve myself, when I have time, because I have to take responsibility for the solution, so I need to thoroughly understand its implementation. So I can't promise to use any code you submit. If you want to keep working on this, I don't object, but please proceed without any expectations.

[ahmed-shariff]

I completely understand your concerns. I couldn't think of a better way to break this down to smaller chunks; as the contraction and expansion functions are very much related. For now, I'll leave this here for now. I myself am using this for my daily tasks. If you need any help with the issues, please feel free to ping me. I'm more than happy to help, I enjoy hacking stuff anywho :)

[alphapapa]

Thank you, that's very kind. You're right that it's not easy to atomize these concerns.

It may be a while before I have time to visit this issue seriously. When I do, I'll study your code more carefully. The tests especially may be a good model.