Export location based consult results to grep buffer.

[artemkovalyov]

I prefer wgrep over occur-edit-mode. I created an alternative exporter for consult-lines to use grep mode buffer.
The benefits are:

• using wgrep and its multi-line results editing capabilities
• uniformity of results editing user experience for grep-like buffers.

I prefer a grep buffer for consult-lines because I use it a lot for narrowing down searches in an open buffer as opposed to grep-based searching tools from consult that constantly search over a directory. In-place editing is richer with wgrep and using one tool makes you ponder less about key combinations and how you enable in-place editing.

Related issues:

• [feature request] Make "*Embark Occur*" supporting wgrep for some special command  #47
• wgrep-mode doesn't work with consult-line export #554

[artemkovalyov]

@oantolin , what would be your recommendation for a more granular configuration to keep other tools like consult-outline, etc. , still using occur. With the current setup, other consult-location based tools will also use grep buffer for exporting. I don't mind that at all but would like to consult about your vision for this.

[minad]

Thank you for this. It is probably a good idea to provide alternative exporters.

However I have two questions or counter points I'd like to see discussed or sorted out first:

1. Are the technical reasons strong enough to justify the addition?

Isearch also exports to occur, we cannot argue the grep/occur distinction away. Embark/Consult and the other packages are somewhat opionated. We try to avoid needless duplications and stay minimal. Therefore I wonder what specific aspects of grep are preferrable for you. Are there strong technical reasons? You can probably configure both occur and grep to behave similarly, if this is only about key bindings. I am kind of sceptical regarding arguments based on taste in contrast to technical reasons. Also note that the grep buffer is weaker out of the box than the occur buffer, if wgrep doesn't exist. Otherwise my suggestion would be to add this to the wiki.

2. Is it reliable given that buffers and files are not the same and not necessarily in sync?

Using grep/wgrep on open buffers worries me a bit and may even lead to inconsistencies between the file and buffer. Are you sure that this behaves correctly? What happens if you use consult-line on a modified buffer and export? What happens then if you wgrep? What happens for buffers which are not backed by a file?

[artemkovalyov]

Thanks for pointing out great questions and detailed write-up, @minad.

I totally support a minimalistic approach and focused nature of the Consult and Embark.

1. Keybninding is not my main concern, but a side-effect. I knew from tools like rg.el how to use wgrep and after trying to use embark with consult it worked in a very similar and highly more interactive fashion with consult-grep. But I use consult-line for a single buffer search most of the time as it's faster and I have to filter through less content. I had to dig into google search and docs to figure out inline editing with occur. I saw I was not alone in asking about it. Then I tried to do a multiline edit in occur and failed. I think it's not supported., is it?
2. The workaround clearly exists - I can use consult-ripgrep and then wgrep for multiline editing when needed. It's just that I like consult-line and use it automatically. I have to redo my searches with grep sometimes when I figure out occur wouldn't do the job for me on the current search-replace occasion. That's a matter of building a habit probably but I decided to write an exporter to consult-line because I didn't see obvious reasons why not.
3. On the edge cases after shallow testing:

• modified buffer - didn't see any side effects. Wgrep worked well, multiline edits were supported. After g you can update your search and re-export. All the changes by wgrep a factored in. Further wgrep updates on those changes are properly applied to the buffer.
• buffer not backed by a file - editing and navigating don't work before you save the buffer. When navigating Emacs prompts for a file. This is not so frequent if an ever existing case for me because for the search-replace situation a buffer almost always exists. I can add a check if a file exists and prompt to save a buffer before running export in that case, as Magit does.
• grep buffer is weaker - for search-replace it's not, same for exporting the data for further processing. It's weaker if you use it to navigate over a buffer, but for that, I'd never use exported occur buffer, I have Consult for that! Do you know cases when people rely on the export to navigate the file? Searching the web I saw that exporting to a buffer is mostly used for search-replace or capturing the data for further processing.

All in all, I can easily survive with grep and occur as opinionated choices, but Emacs is a bit about fine-tuning and this is a good tuning for me. I would prefer to have the option to configure where I want to export to accommodate my working pipeline.
I think occur is a great default option. I'm happy to discuss how to make a reasonable overriding mechanism if a grep buffer is preferred. Other location-based Consult tools worked also worked fine with the exporter. See the consult-outline example.

I think adding a check about saved buffer, in that case, is a good idea in a similar fashion Magit does it.

I'm happy to polish the PR and add more docs if you and @oantolin will consider the exporter useful. Otherwise, I'll maintain it in my configs as my own little hack and pray for no bigger API changes:)

Offtopic:
What do you use for search-replace?

[oantolin]

I'll read through this conversation carefully later, but my first reaction is that it doesn't really make sense to have consult-line export to a grep-mode buffer because consult-line is meant to search through a buffer whether or not that buffer is visiting a file (and I, for one, do use it very often on non-file buffers!); grep-mode on the other hand is meant to list results only in files. So my first instinct is to reject this PR on semantic grounds: it seems actually incorrect to me to do this.

Of course, if you do not want the capability of exporting consult-line results from non-file buffers, you are free to reject this capability in your personal configuration, and since others may also want to forgo this feature in exchange for grep-mode it would be nice to have this documented in the wiki.

[oantolin]

On second thought, if we keep occur as the default and document the limitations of exporting to grep, I don't see why we couldn't keep both exporters.

[oantolin]

OK, I'm reading the conversation here now (which is not as long as I thought it was, there is a large screenshot which made me overestimate the length :)).

what would be your recommendation for a more granular configuration to keep other tools like consult-outline, etc. , still using occur

I'd suggest using marginalia to have consult-line report a different category, say consult-line-location, and configure the exporter only for that.

I see @minad brought up the same concern I have.

I'm also curious about what is better about wgrep compared to occur-edit-mode. I've always thought of wgrep as implementing occur-edit-mode but for grep. You mention "multiline edits", but I'm not sure what you mean by that, I can think of two meanings but both work fine here:

1. Maybe you meant editing an occur buffer containing multiline results (as obtained by calling occur with a numeric prefix parameter specifying the number of context lines). That works here just fine, and it is probably not what you meant since there is no way to get embark export to give you context lines.

2. Maybe you meant editing several different single-line results in an occur-mode buffer. That also works perfectly well here and I would say is probably the main use case for occur-edit-mode. It would be weird if that didn't work.

So either something is a little off in your configuration (and we can certainly try to figure out what), or you meant a third thing by "multiline edit".

Continuing, you say "I had to dig into google search and docs to figure out inline editing with occur", which is a perfectly valid way of finding out about it, but I want to point out that: (1) that would be the same for wgrep, people need to find out about it somehow in order to use it; (2) for wgrep finding out necessarily involves the internet, but occur-edit-mode is built-in and well-documented inside Emacs, so no googling is required to find it.

I'm glad you looked into @minad's questions, and your answers would form the backbone of documenting the pitfalls of using grep-mode export for consult-line. I do have one minor correction, you say "grep buffer is weaker - for search-replace it's not". But it definitely is weaker at least in the sense that editing requires the third-party wgrep package, while occur-edit-mode is built-in.

I think that on balance, the first thing to figure out is why whatever you meant by "multiline edits" are not working for you with occur-edit-mode. If we fix that you may not even feel a need for grep export anymore.

[minad]

My initial reaction was also to reject because of the separation grep/occur and file/buffer.

However if grep+wgrep work well and the grep export can be seen as a generalization of the occur export somehow. Then why not? However I see it as a requirement that we don't introduce bugs by adding this (handling non file backed buffers, buffer/file out of sync, ...). The exporter must be sufficiently robust in most (or all) scenarios.

Not sure if we had this discussion before but maybe we also need a way to select between multiple exporters.

[minad]

@oantolin Iirc there was a feature request for export with context lines to occur.

[oantolin]

Do you know cases when people rely on the export to navigate the file?

Forgot to answer this: I do that fairly often. I use it when I need to make a bunch of changes everywhere some string appears, for example, if I'm programming and changed a function signature and need to consider how best to update each call-site, or if I'm writing some math and decided to change the name of a concept I'm defining. What I like about using occur for these situations is that the occur buffer works like a temporary todo list containing all the spots where I need to change something; and I can navigate among the spots with next-error and previous-error, I don't even need to switch to the occur mode buffer.

Of course, in these use cases I'm editing an actual file so grep would do just as well. But I do sometimes use consult-line in the list-packages buffer and export to an occur-mode buffer which I treat as a todo list of packages I wish to check out (I do this when I want to search both package names and one-line descriptions, for just packages names I prefer to use describe-package). But this use case could easily be replaced by consult-focus (which I also sometimes use for the same purpose, I'm inconsistent that way).

[oantolin]

@minad said:

Not sure if we had this discussion before but maybe we also need a way to select between multiple exporters.

You mean selecting at export time? That could be a nice to have. A first idea on the UI would be that if (alist-get embark-exporters-alist type) is a list (whose car is not lambda or closure... 🙄), then you get prompted for which exporter you want. But it may be premature to do that, we seem to have managed with a single "best" exporter for each type of targets so far. In this particular case, I'm still not sure I understand why occur-edit-mode isn't good enough for @artemkovalyov.

Iirc there was a feature request for export with context lines to occur.

Yes, there was, and I do think that's a good idea, I just forgot about it. I guess a first step would be to pass the prefix argument to the exporter function somehow, probably just as the value of prefix-arg (which I think currently gets reset to nil before the exporter even runs).

[minad]

@oantolin Yes, I'd also like to understand the real value of this addition better and why @artemkovalyov isn't happy with occur and occur-edit-mode. I don't think you can "run away" from occur given that it is also baked into isearch. Just adding a grep export because we can and because of some consistency details doesn't seem justified (we have the wiki for that), considering that we introduce new problems given non-file backed buffers and given that wgrep is needed to reach parity with occur.

[artemkovalyov]

@oantolin, I think you didn't get what multi-line edit means. It's about the ability to add lines in an exported buffer that are committed to the original buffer (file).

Here are examples with Wgrep and occure-edit

• In Wgrep new lines are handled perfectly well and that's a very useful feature (at least to me)
  

• In occur-edit I can only edit the exported line, I can't remove it or add new lines
  

[minad]

@artemkovalyov That's indeed an interesting feature! I wasn't aware of that. I think occur-edit-mode should be improved such that this is also supported.

[oantolin]

You're absolutely right that I hadn't understood what you meant, @artemkovalyov! (I suspected as much.) That is a cool feature I did not know wgrep had! Like @minad, I think the right approach is to enhance occur-edit-mode to support this, but in the meantime, I do think that for that feature it is worth having the grep exporter even if it only works properly for file-backed buffers.

[minad]

Is there some code here which could be shared with embark-consult-export-grep? Maybe one could introduce a helper function embark-consult--export-grep which sets up the grep buffer is passed a function as argument. The function argument should insert the actual content into the buffer.

(defun embark-consult--export-grep (insert)
  (let ((buf (generate-new-buffer "*Embark Export Grep*")))
    (with-current-buffer buf
      (insert (propertize "Exported grep results:\n\n" 'wgrep-header t))
      (funcall insert)
      (goto-char (point-min))
      (grep-mode)
      ;; Make this buffer current for next/previous-error
      (setq next-error-last-buffer buf)
      ;; Set up keymap before possible wgrep-setup, so that wgrep
      ;; restores our binding too when the user finishes editing.
      (use-local-map (make-composed-keymap
                      embark-consult-revert-map
                      (current-local-map)))
      (setq-local wgrep-header/footer-parser #'ignore)
      (when (fboundp 'wgrep-setup) (wgrep-setup)))
    (pop-to-buffer buf)))

(defun embark-consult-export-grep (lines)
  ...)

If we share the code, this addition will be very small. We don't add complexity and avoid complicating maintenance.

[oantolin]

I think that instead of the function you propose @minad, with an insert function argument, that this new export function should just build a grep-formatted list of lines and then call the existing embark-consult-export-grep.

[minad]

I thought a callback would be more efficient, since we can write directly to the buffer. Building an intermediate list requires allocations, which can be costly if we have many candidates. It would be even worse to build many intermediate strings.

[oantolin]

You're right, of course. I just thought it shouldn't be a problem since I'd expect few matching lines compared to the number of lines in the buffer, so the slow step would actually be the initial gathering of candidates in consult-line. I mostly wanted to reuse more code, in particular to reuse the line insertion loop, so maybe the best option is a callback but one that is called per line.

[artemkovalyov]

@minad , @oantolin , I'll consider your suggestions to better integrate this exporter into the library. I'm now testing a slightly better design of the exported results based on how rg.el does it.

[minad]

Any ideas for a better name?

[artemkovalyov]

I'll update the name after refactoring. This was my "code name" because I was looking for a solution specifically for lines. I didn't draw much difference from consult-location group back when I started my initial solution discovery.

[minad]

On 11/22/22 18:10, Omar Antolín Camarena wrote: ***@***.**** commented on this pull request. ------------------------------------------------------------------------ In embark-consult.el <#560 (comment)>: > + (contents (propertize (embark-consult--strip line))) + (this-buf (marker-buffer loc))) + (unless (eq this-buf last-buf) + (insert (propertize "Exported grep results:\n\n" 'wgrep-header t)) + (setq last-buf this-buf)) + (insert (concat (file-name-nondirectory (buffer-file-name this-buf)) ":" lineno contents "\n")))) + (goto-char (point-min)) + (grep-mode) + (setq next-error-last-buffer buf) + ;; Set up keymap before possible wgrep-setup, so that wgrep + ;; restores our binding too when the user finishes editing. + (use-local-map (make-composed-keymap + embark-consult-revert-map + (current-local-map))) + (setq-local wgrep-header/footer-parser #'ignore) + (when (fboundp 'wgrep-setup) (wgrep-setup))) You're right, of course. I just thought it shouldn't be a problem since I'd expect few matching lines compared to the number of lines in the buffer, so the slow step would actually be the initial gathering of candidates in consult-line. I mostly wanted to reuse more code, in particular to reuse the line insertion loop, so maybe the best option is a callback but one that is called per line.

Hmm, I would probably go with the most efficient solution, a single call. I often export many candidates from consult-grep/ripgrep. As an alternative we could also use a macro with a BODY.

[artemkovalyov]

Thanks for the productive discussions. I'll work on updating the PR.

[artemkovalyov]

@minad , @oantolin , I'll consider your suggestions to better integrate this exporter into the library. I'm now testing a slightly better design of the exported results based on how rg.el does it.

[artemkovalyov]

I'll update the name after refactoring. This was my "code name" because I was looking for a solution specifically for lines. I didn't draw much difference from consult-location group back when I started my initial solution discovery.

[oantolin]

Oh, @artemkovalyov have you signed the FSF copyright assignment papers?

[artemkovalyov]

Oh, @artemkovalyov have you signed the FSF copyright assignment papers?
I assume there's no GH plugin to do that? Do you have a simple instruction or link? Googling confuse me :)

[artemkovalyov]

@minad, @oantolin, please, take another look at the implementation. I'm not an experienced LISPer but I tried to follow your recommendations. I had to create wrappers for actual exporters to preserve their APIs but split the implementation into a buffer setup and processing of results to make them reusable.

If you have better ideas, please, share. I also renamed an exporter to occur into a location, as the destination buffer can now be different. It seems to play nicely with the naming strategy and everything before that also uses location for the range of location-enabled consult functions.

[artemkovalyov]

@minad, I made sure the buffers exported to grep buffer are visiting a file. Here are a couple of screenshots:


All the temporary buffers like scratch and others will be excluded from consult-line-multi for example.
After we agree on the implementation and details I'll add some WIKI about this exporter.
The switch of the behavior is done via defcustom var for now.

[oantolin]

Sure, if you don't have time to fix the issues in my code review, I can do that for you, I just wanted to give you a chance to do that first, @artemkovalyov. Or did you fix them and just not mark them as done? I'll check. EDIT: It seems like you still haven't fixed them.

[oantolin]

This uses the variable buf, but is outside the let.

[oantolin]

I think you don't use this variable at all.

[oantolin]

add-to-list isn't meant to work on lexical variables, use push (handling duplicates yourself) or cl-pushnew.

[oantolin]

Oh, boy, I think I had not submitted the review so that means that all these months you hadn't even seen it! (If I recall correctly I did the review at the end of April after minad's comment.) I'm very sorry, it's the first time I ever do a code review (I'd always used the "add a single comment" feature before), and I did not know I had to submit it or where to click to submit.

[artemkovalyov]

@oantolin, it happened to the best of us :) I'll take a look at your comments and suggestions. It's so sad I'm not coding LISP daily although using Emacs extensively.

[minad]

Any news on this one?

[oantolin]

Not on my end. I'm stil hoping @artemkovalyov fixes those issues I brought up.

[minad]

I suspect that @artemkovalyov followed your example and forgot to click the "submit commits" button. ;)

[oantolin]

Well, I didn't forget to submit: I didn't know you had too! And also didn't notice the submit button. 😬 I don't know if this is better or worse than forgetting to submit when you know you have to.

[artemkovalyov]

Hey @oantolin & @minad, I resolved the conflicts and fixed minor issues that were pointed out by @oantolin.
I like your humor in the comments. The actual reason is my two three year old twins. They are making coding for fun possible only when they are asleep :)

[artemkovalyov]

@oantolin, I took a quick look at how to provide a choice of a desired exporting format for consult-location but failed to find a nice pattern. My idea was to have a list of exporter functions on mapping alist and prompt about which one to use. This seems to interfere with your design quite a bit and needs quite some refactoring.

With your expertise, I guess you can propose a beautiful solution here. I can attempt another PR then if this one is accepted and I could figure that out.

Otherwise, as before one can choose their exporter via config and keep it stupid simple.
(setf (alist-get 'consult-location embark-exporters-alist) #'embark-consult-export-location-grep)

[artemkovalyov]

Tested for both single and multi-buffer case

[artemkovalyov]

I'm feeling super dull, but I didn't manage to figure out how to make a function that would create a prompt based on a defined keymap. All those targets and indicators are getting me lost...

[oantolin]

Thanks for fixing the things I pointed out! And congratulations on the twins, I can certainly understand that they make hobby coding a very low priority, since I went through the same thing (with one half of the number of toddlers).

I took a quick look at how to provide a choice of a desired exporting format for consult-location but failed to find a nice pattern. My idea was to have a list of exporter functions on mapping alist and prompt about which one to use. This seems to interfere with your design quite a bit and needs quite some refactoring.

I don't think that's a good idea or even necessary, I think most people will always want occur export or always want grep, so it's much better as a setting in the user's configuration. The code you gave is exactly what I had in mind:

(setf  (alist-get 'consult-location  embark-exporters-alist) #'embark-consult-export-location-grep)

I'll test the new code later today and merge. Thanks again!

[oantolin]

There is a whole lot of duplication between embark-consult-export-grep and embark-consult-export-location-grep. I will refactor as we suggested above. (I'll fix a couple of bugs I spotted in the new function along the way: one is that it doesn't correctly gather the list of non-file-backed buffers, another that it doesn't handle newer versions of wgrep which changed a variable name.)

[oantolin]

Another small issue: the new function does not count the matches so it does not set the grep-num-matches-found variable. I don't know if that is actually important or not, but we might as well set it to the correct value.

[oantolin]

I like that the new function prints the file names invisibly and has a header for each file that is visible but marked as ignored by wgrep.

EDIT: On second thought, I don't like that: it makes it too unlike the consult-grep exporter and the M-x grep family of commands. I'll make it more standard.

[oantolin]

OK, I'm done with my changes. It looks ready to merge on my end. I'll maybe test a little bit more, then merge.

[oantolin]

OK, done with testing. Will merge now.

[artemkovalyov]

Thank you for the collaboration, @oantolin, and for bothering to create embark :)