Closed Ambrevar closed 6 years ago
thierryvolpiatto
commented
6 years ago Pierre Neidhardt notifications@github.com writes:
With vanilla's occur', callingnext-errororprevious-errorallows for navigating through the results.helm-occur` does not seem to support that. Would it be possible to add this feature?
No, I am against this feature unuseful in a typical helm workflow.
-- Thierry
Ambrevar
commented
6 years ago Can you detail this workflow?
thierryvolpiatto
commented
6 years ago Pierre Neidhardt notifications@github.com writes:
Can you detail this workflow?
helm-resume+follow-mode is much more efficient than using next/previous error blindly n or -n times to finally jump on what you want.
If you really want this see: https://github.com/sameerds/helm-modeless
-- Thierry
Ambrevar
commented
6 years ago I think it's a good rationale. Thanks for your input!
x-yuri
commented
3 years ago @thierryvolpiatto Can you possibly elaborate for a person new to Helm (and well... to a lesser extent to Emacs)? What use case are you talking about? What approach is bad? What is good?
I think next-error/previous-error are useful when you want to inspect where some identifier is used (think rgrep). Particularly because they're available in other buffers. Sometimes, it makes sense to inspect the grep buffer and open select files. Sometimes, it's rather in the way, and M-g M-n/M-g M-p come in handy. Is this approach bad? Why? What is a better way?
Perhaps, it makes sense to mention where this is coming from.
thierryvolpiatto
commented
3 years ago x-yuri @.***> writes:
I think next-error/previous-error are useful when you want to inspect where some identifier is used (think rgrep). Particularly because they're available in other buffers. Sometimes, it makes sense to inspect the grep buffer and open select files. Sometimes, it's rather in the way, and M-g M-n/M-g M-p come in handy. Is this approach bad? Why? What is a better way?
This approach is not bad but IMO deprecated once you use helm and ripgrep. It is so fast to just call helm-grep from helm-find-files or from the buffer you are working on that you don't need next-error (I personally never used it since I use helm (2008)). This is the reason why I never implemented next-error for helm, however you can save a helm-grep buffer and reuse it if needed (I use this mostly to edit it with wgrep), again reaching the buffer is very easy with helm-mini or if the grep search is local to the project you are working on with M-x helm-browse-project.
-- Thierry
x-yuri
commented
3 years ago I'm trying to understand how that works for you. Today I had to inspect if timeout specified in redis.yml is ever used. Seeing only lines where redis.yml is mentioned won't answer that question. I had to open each file that references redis.yml and inspect the surrounding code. I don't see how I could figure that out without opening each file, that references redis.yml. And you supposedly... don't run into such tasks?
The 2 ways I know that could help me with this is: 1) rgrep, but it doesn't use rg, so doesn't ignores the gitignored files (but probably could be made to do so), 2) projectile + rg.
About wgrep, it doesn't work with Helm's grep bufffer. It says, "C-c C-p is undefined." Or I'm using some other wgrep. The one I use is this one:
Installed via M-x package-install Enter wgrep Enter.
thierryvolpiatto
commented
3 years ago x-yuri @.***> writes:
I'm trying to understand how that works for you. Today I had to inspect if timeout specified in redis.yml is ever used. Seeing only lines where redis.yml is mentioned won't answer that question. I had to open each file that references redis.yml and inspect the surrounding code. I don't see how I could figure that out without opening each file, that references redis.yml. And you supposedly... don't run into such tasks?
I don't see the relation with next-error, but I am not sure to understand what you are doing.
About wgrep, it doesn't work with Helm's grep bufffer. It says, "C-c C-p is undefined." Or I'm using some other wgrep. The one I use is this one: https://melpa.org/#/wgrep
The package for helm is wgrep-helm which install as dependency wgrep.
And it is working fine here.
-- Thierry
x-yuri
commented
3 years ago I see.
About next-error. I was trying to provide an example of a situation where I needed it. next-error has to do with it because it allows to navigate between occurrences without switching to the grep buffer (available in every buffer via M-g M-n). For example, after doing rgrep I can close the window with the grep buffer, and I'll still be able to switch between occurrences.
Regarding my example, I was working on a Ruby project. I needed to find out if timeout specified in redis.yml was ever used. For that I needed to find every place where redis.yml is opened and examine the surrounding code. What I'm asking is how I can achieve that without opening each such file. Okay, I can do it via the hgrep buffer. That is, Enter to visit the first file, then back to the hgrep buffer, then Enter on the next file, and so on. But it seems more convenient to me to just use M-g M-n/M-g M-p.
Or another example, I needed to inspect all the places in the project where Time.now is called. I couldn't just replace Time.now with Time.current because I was not sure what to do back then. And as it turned out there were some cases, where such change is inappropriate.
Don't you ever run into situation where you need to inspect every place where some variable is used? And for that you need to open every such file? From what you said so far, it seems like the answer is "no." Which sounds strange. Or maybe I'm doing something wrong? I don't know, overly cautious. Or maybe there's some way to solve similar problems that I can not see?
thierryvolpiatto
commented
3 years ago x-yuri @.***> writes:
I see.
About next-error. I was trying to provide an example of a situation where I needed it. next-error has to do with it because it allows to navigate between occurrences without switching to the grep buffer (available in every buffer via M-g M-n). For example, after doing rgrep I can close the window with the grep buffer, and I'll still be able to switch between occurrences.
Regarding my example, I was working on a Ruby project. I needed to find out if timeout specified in redis.yml was ever used. For that I needed to find every place where redis.yml is opened and examine the surrounding code. What I'm asking is how I can achieve that without opening each such file. Okay, I can do it via the hgrep buffer. That is, Enter to visit the first file, then back to the hgrep buffer, then Enter on the next file, and so on. But it seems more convenient to me to just use M-g M-n/M-g M-p.
Or another example, I needed to inspect all the places in the project where Time.now is called. I couldn't just replace Time.now with Time.current because I was not sure what to do back then. And as it turned out there were some cases, where such change is inappropriate.
Don't you ever run into situation where you need to inspect every place where some variable is used? And for that you need to open every such file? From what you said so far, it seems like the answer is "no." Which sounds strange. Or maybe I'm doing something wrong? I don't know, overly cautious. Or maybe there's some way to solve similar problems that I can not see?
I think what you want is persistent action from a helm buffer (C-j) and you don't have to save helm buffer.
-- Thierry
x-yuri
commented
3 years ago Indeed, I didn't understand persistent actions. That helps in some cases. And thanks for the suggestions so far. But after all it doesn't cover all my use cases. You say you edit using wgrep. Does it allow to delete lines? What if you can't just delete the line. For example:
RUN apk add --no-index build-base \
gitTo delete the "git" line, you've got to:
-RUN apk add --no-index build-base \
+RUN apk add --no-index build-base
- gitOkay, you can probably edit what you can in the grep buffer, then edit the rest of the files one-by-one (Enter, edit, C-k).
And persistent actions doesn't always help. Sometimes you might want to see not just the part of the file near the occurrence, but inspect the whole file. Okay, you can just open the file, inspect it, then helm-resume the session. But you have to be careful, not to start a new one.
For now, I'll try to stick to the suggested workflows, but at the moment it doesn't look like it's always the best option.
thierryvolpiatto
commented
3 years ago x-yuri @.***> writes:
- ( ) text/plain (*) text/html
Indeed, I didn't understand persistent actions. That helps in some cases. And thanks for the suggestions so far. But after all it doesn't cover all my use cases. You say you edit using wgrep. Does it allow to delete lines?
Yes.
And persistent actions doesn't always help. Sometimes you might want to see not just the part of the file near the occurrence,
You can scroll up and down PA window (C-M <down/up>).
but inspect the whole file.
If you want to inspect the whole file I don't see the point of starting a grep/occur tool?
-- Thierry
x-yuri
commented
2 years ago You can scroll up and down PA window (C-M <down/up>).
Oh, wow, indeed, gotta give it a try.
If you want to inspect the whole file I don't see the point of starting a grep/occur tool?
I meant a case where you want to find out where and how a variable is used. In multiple files. Sometimes it's not enough to see the line, or just the surrounding lines. I have to try "C-M <down/up>" next time.
x-yuri
commented
2 years ago This approach is not bad but IMO deprecated once you use helm and ripgrep.
And after all I beg to disagree. Most of the time there are better ways, but occasionally I need next-error/previous-error. E.g. I now need to replace:
div(
data-ng-include=""
src="'some/path'"
)with:
ng-include(
src="'some/path'"
)across a project. The natural thing is to look for ng-include, but then I'm forced to one of the approaches:
RET, make the changes, switch back to *hgrep*.RET, make the changes, helm-resume.With next-error I could do: M-g n, make the changes, M-g n, ...
thierryvolpiatto
commented
2 years ago x-yuri @.***> writes:
With next-error I could do: M-g n, make the changes, M-g n, ...
And you will be 1+ line wrong after each replacement without any way to refresh.
-- Thierry
thierryvolpiatto
commented
2 years ago x-yuri @.***> writes:
This approach is not bad but IMO deprecated once you use helm and ripgrep.And after all I beg to disagree. Most of the time there are better ways, but occasionally I need next-error/previous-error. E.g. I now need to replace:
div( data-ng-include="" src="'some/path'" )
with:
ng-include( src="'some/path'" )
across a project. The natural thing is to look for ng-include, but then I'm forced to one of the approaches:
For such a task, I suggest using the excellent Iedit.
-- Thierry
x-yuri
commented
2 years ago Okay, let's be more specific. Consider this diff (I've slightly strayed from the way I wanted to change the code originally, but that doesn't change the point):
As you can see it's not as straightforward as I made it look above (the modifications differ slightly from case to case).
And you will be 1+ line wrong after each replacement without any way to refresh.
With wgrep? Yes. With projectile? No. It appears to readjust the line numbers.
But considering wgrep, what the alternative approach would be? Provide a pattern that matches the sibling lines? I'm not sure how viable that is. As a result it might match a lot of extra lines.
The way I see it, sometimes projectile is a better tool for the job. Which doesn't imply that helm is bad.
iedit? That's an interesting suggestion. It can probably help here to some extent. But for now it looks like projectile + next-error + macros for the common cases sounds like a better option. Because although most of the changes in the diff follow one of several patterns, but there are also some exceptions. Or alternatively, there's one common pattern and exceptions. With next-error I have to consider each case separately (apply a macro or edit manually), with iedit I have to think how many occurrences to change at once (more thinking). And although I'm new to iedit I don't see a way to save a renaming under some name and reapply it to an arbitrary number of occurrences later.
You might say that I can split it into several commits, but even then I think wgrep and iedit doesn't work well with multiline changes.
thierryvolpiatto
commented
2 years ago x-yuri @.***> writes:
You might say that I can split it into several commits, but even then I think wgrep and iedit doesn't work well with multiline changes.
Wgrep doesn't but iedit works fine in this case (mark the region you want to delete).
Never mind, I made a next-error-function for helm-grep-mode buffer, will do the same for helm-occur-mode buffer, it is not published yet but will be available soon. It is working fine with helm-grep-mode, only caveat, if you modify the buffer where you jump, the helm-grep buffer is not updated, will try to fix this later.
-- Thierry
thierryvolpiatto
commented
2 years ago Here the diff if you want to try it:
commit 9aa6b83f19d493d1b7c31d567bbf26780315c250
Author: Thierry Volpiatto <thievol@posteo.net>
Date: Wed Feb 9 09:31:57 2022 +0100
Add next-error-function to helm-grep-mode #2065
diff --git a/helm-grep.el b/helm-grep.el
index c20fb759..0c91babd 100644
--- a/helm-grep.el
+++ b/helm-grep.el
@@ -874,6 +874,18 @@ If N is positive go forward otherwise go backward."
(helm-grep-mode-jump)))))
(put 'helm-grep-mode-mouse-jump 'helm-only t)
+(defun helm-grep-next-error (&optional argp reset)
+ (interactive "p")
+ (goto-char (cond (reset (point-min))
+ ((< argp 0) (line-beginning-position))
+ ((> argp 0) (line-end-position))
+ ((point))))
+ (let ((fun (if (> argp 0) #'next-single-property-change #'previous-single-property-change)))
+ (helm-aif (funcall fun (point) 'helm-grep-fname)
+ (progn (goto-char it) (helm-grep-mode-jump))
+ (user-error "No more matches"))))
+(put 'helm-grep-next-error 'helm-only t)
+
(define-derived-mode helm-grep-mode
special-mode "helm-grep"
"Major mode to provide actions in helm grep saved buffer.
@@ -883,7 +895,9 @@ Special commands:
(set (make-local-variable 'helm-grep-last-cmd-line)
(with-helm-buffer helm-grep-last-cmd-line))
(set (make-local-variable 'revert-buffer-function)
- #'helm-grep-mode--revert-buffer-function))
+ #'helm-grep-mode--revert-buffer-function)
+ (set (make-local-variable 'next-error-function)
+ #'helm-grep-next-error))
(put 'helm-grep-mode 'helm-only t)
(defun helm-grep-mode--revert-buffer-function (&optional _ignore-auto _noconfirm)
Working now for helm-occur-mode and merged now.
thierryvolpiatto
commented
2 years ago NOTE: Other tool to consider to navigate from one change to the other is git-gutter. e.g. Make a first change on each block of code which will be common for all these blocks in a buffer with Iedit and then jump to each occurences with git-gutter to make more specific changes.
If you use next/previous-error from a saved helm-grep/occur-mode buffer you will have for now to refresh this buffer from time to time with "g".
thierryvolpiatto
commented
2 years ago x-yuri @.***> writes:
And you will be 1+ line wrong after each replacement without any way to refresh.With wgrep? Yes. With projectile? No. It appears to readjust the line numbers.
I hardly see how this would happen, I don't see in the projectile code anything that readjust line numbers. Though I never used projectile, only reading source code for the purpose, so I may missed something.
Now Helm have a function to revert the grep buffer from elsewhere (while navigating with next error). So AFAICT helm is the only tool that allows readjusting linums.
Here the workflow:
1) Make your search with helm-grep (rg or whatever). 2) Save helm-grep buffer. 3) Hit RET to reach first occurence. 4) Make your change. 5) C-x C-s and M-x helm-revert-next-error-last-buffer (bind it to a convenient key, here M-g M-g). 6) next-error, make your change and so on.
NOTES: If you don't modify lines numbers, you don't have to revert buffer, next-error will bring you more or less to the right place.
With helm-occur which act on buffer and not files the workflow is the same but you don't have to save buffer to its files before reverting.
-- Thierry
With vanilla's
occur, callingnext-errororprevious-errorallows for navigating through the results.helm-occurdoes not seem to support that. Would it be possible to add this feature?