cosmicexplorer / helm-rg

ripgrep is nice
GNU General Public License v3.0
101 stars 21 forks source link

Make interface consistent w/ `helm-grep-do-git-grep` and `helm-do-ag` #2

Closed xiongtx closed 6 years ago

xiongtx commented 6 years ago

Thanks for creating this! We're using it in helm-projectile as the Ripgrep interface, but unfortunately it's not quite the same as other search interfaces. See this comment.

Here's helm-rg:

helm-rg

image

And -grep and -ag commands, from Helm and helm-ag respectively:

helm-grep-do-git-grep

image

helm-do-ag

image

It'd be nice if helm-rg could follow the same design as the others for a consistent user experience!

cosmicexplorer commented 6 years ago

Thanks for filing this issue!

I actually just changed the display of each line to its current form last week or so. While I introduced several other "architectural" changes during that development, I will hopefully be able to copy back the previous display code and allow switching between the display modes with a defcustom named maybe helm-rg--results-display-style. I would like to keep the default value for this defcustom to use the new file/line display interface, but the other value would be a symbol maybe named 'with-file-location. Your code could then call it as:

(let ((helm-rg--results-display-style 'with-file-location))
  (helm-rg "" nil (list (projectile-project-root))))

or however else you might want to do that.

xiongtx commented 6 years ago

Sounds good--let me know when you'd like me to review / try it out.

cosmicexplorer commented 6 years ago

@xiongtx as of 2f1ea6db272311f0aae99512a1a70e3e3286393a, you should be able to use the following to achieve the desired output:

(let ((helm-rg-prepend-file-name-line-at-top-of-matches nil)
      (helm-rg-include-file-on-every-match-line t))
  (helm-rg "" nil (list (projectile-project-root))))

wow2

Please let me know if that works for your needs.

xiongtx commented 6 years ago

Thanks! I've pushed a commit.

xiongtx commented 6 years ago

Small issue: it seems the file paths are always fully expanded, which takes a lot of space. It'd be nice if they were expanded against the base directory. This is the case for helm-ag etc.

image

cosmicexplorer commented 6 years ago

@xiongtx Thanks for describing the issue! I couldn't think of a situation when I would want the absolute paths instead of relative paths, so a37c53f28c095164b77ce10264bf0b777316599b should do as expected (using the path relative to the executing directory), see: wow3

Let me know if this seems to work for you.

xiongtx commented 6 years ago

Yup, that'll do it! Thanks!

jamesnvc commented 5 years ago

I'm finding that the last change above, to show relative paths, makes helm-resume not work properly, because when the search resumes it will try to resolve the relative paths to the current directory; reverting a37c53f makes it seem to work properly for me; would it be possible to make that an option or something?

cosmicexplorer commented 5 years ago

Yes, it should have been an option in the first place! Will do. EDIT: Well, there's a right way to do this that would actually solve the problem and resolve them relative to the correct directory, but I will leave that as a TODO for now.

cosmicexplorer commented 5 years ago

@jamesnvc please let me know if 2326451 (introducing helm-rg-file-paths-in-matches-behavior, which can be set to 'absolute) resolves your issue, and if not, could you open a new issue? Thanks for the note!

jamesnvc commented 5 years ago

@cosmicexplorer perfect, that's great! Thank you very much :smile:

cosmicexplorer commented 5 years ago

@jamesnvc I've also seen #14 and added a test framework so it won't break so when that gets merged you may be able to revert that customization!