alphapapa / dogears.el

Never lose your place in Emacs again
GNU General Public License v3.0
190 stars 7 forks source link

Provide multi-line context #20

Open edgar-vincent opened 6 months ago

edgar-vincent commented 6 months ago

Hi Adam,

Thanks a lot for this very useful package.

Do you think it would be possible to, somehow, display a few lines of context relative to each 'dogear' (much like a diff file does)? I find it quite difficult to find my way around without the context. I know one can customise 'dogears-line-width', but sometimes a single line of context, however long it is, doesn't make it easy enough to quickly identify the location I am looking for.

I assume this would not be possible to implement in dogears-go because of the use of the minibuffer. Perhaps a specific command/display mechanism would be needed?

What do you think?

Thanks a lot!

Edgar

alphapapa commented 6 months ago

Hi Edgar,

That's an interesting idea. I'm sure something like that could be done. I'm open to the idea, but I don't expect I'll have time to work on it myself.

Thanks.

edgar-vincent commented 6 months ago

Great! I'll see what I can do! It might take a while, because I'm not a particularly advanced elisp hacker.

alphapapa commented 6 months ago

No problem. FWIW, you'll likely want to look at how Helm and Consult offer multiline completion candidates. I'm not sure if there's a practical way to do multiline candidates using Emacs's own completion APIs.

edgar-vincent commented 5 months ago

I started a draft PR which attempts to implement this feature, but I have no idea what it's worth: https://github.com/alphapapa/dogears.el/pull/21

hab25 commented 2 months ago

No problem. FWIW, you'll likely want to look at how Helm and Consult offer multiline completion candidates.

As a user, for me the best solution would be to have a consult-dogears command.

If there is a concern about putting an unnecessary dependency requirement on users, this integration could be optional, perhaps similarly to how embark does it (see https://github.com/oantolin/embark/blob/d3c9d1b4c890cf365846cc2b418f37341999e79f/embark.el#L4597C1-L4599C97).

Speaking of embark-consult, if the above were implemented, by using that could one could easily get a dogears-list-mode-like buffer "for free" by commanding embark-export during consult-dogears. Note that feature parity would probably require that consult-dogears appropriately set completion-extra-properties with all the same information made available by dogears-list-mode (i.e. the data shown in the "Relevance", "Within", etc columns).

Personally, if I were maintaing dogears and were lacking on time, I would probably be quite excited about getting rid of all code that exists exclusively to support dogears-list-mode, instead recommending users install these dependencies and use these commands.

edgar-vincent commented 2 months ago

In the case of this specific issue, the problem is that consult cannot provide multi-line candidates.

hab25 commented 2 months ago

In the case of this specific issue, the problem is that consult cannot provide multi-line candidates.

Right, instead a preview buffer would be displayed when one "hovered" over a candidate of consult-dogears. I consider that to be superior because 1) one only gets the multi-line context for the single candidate they're currently considering, which allows for 2) showing an "entire buffer" of multi-line context at once.

hab25 commented 2 months ago

In case it isn't clear: I therefore don't consider that to be a downside.

alphapapa commented 2 months ago

In the case of this specific issue, the problem is that consult cannot provide multi-line candidates.

To be more specific, AFAICT it can, but it collapses multiple lines (e.g. https://github.com/minad/consult/issues/187).

I don't have a strong opinion about these ideas. If someone wants to propose a patch, like #21, that's fine. But I think that we should try to implement as much as we can without relying on third-party libraries (i.e. if Dogears is to provide multi-line context, it should work for more than just the Helm frontend).

hab25 commented 2 months ago

relying on third-party libraries

N.b. consult is in GNU ELPA.

But I think that we should try to implement as much as we can without relying on third-party libraries

I think the consult case would share so little code with the vanilla implementation that it doesn't make sense to pull-request them together.

if Dogears is to provide multi-line context, it should work for more than just the Helm frontend

What is the benefit of this constraint? In my view there is none: in the case that only a Helm implementation exists, because nobody is interested in implementing vanilla, rejecting the former means Helm users lose out while vanilla users are indifferent.