Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finder for symbols in the help buffer #595

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Hugo-Heagren
Copy link

I wrote this today as a convenience for myself, and realised others might also use it. It's a target finder which locates the symbol a help buffer describes, and makes heuristic guesses about what type of thing it is. It isn't perfect, but it might be useful for someone.

@minad
Copy link
Contributor

minad commented Feb 9, 2023

@Hugo-Heagren Let me say first that I am sorry that every time you make a suggestion, I appear with some critical remarks. The problem is that I have a (too?) narrow picture about Embark in my mind.

It appears that the target finder you wrote here applies to the symbol relevant for the whole buffer. In this sense it is similar to @oantolin's old idea of introducing a buffer target, which matches everywhere in the whole buffer and returns the buffer itself as target. See https://github.com/oantolin/embark/wiki/Additional-Actions#use-embark-like-a-leader-key. Back then it was decided that such buffer actions are not localized enough and that a local keymap is the better solution for buffer-wide keybindings.

A similar criticism applies to your target finder, given that it is not localized. You are not truly acting at point. Instead your target finder works like a catch all mechanism which matches when no other more local target matches. This behavior is quite different from other target finders. My problem is that with such catch-all finders the targets somehow lose their meaning since everything is a target suddenly. Therefore I argue that catch-all target finders like this one are not the ideal fit for Embark and should maybe put into the wiki instead.

Other opinions? Thoughts?

@Hugo-Heagren
Copy link
Author

Hugo-Heagren commented Feb 9, 2023

Let me say first that I am sorry that every time you make a suggestion, I appear with some critical remarks.

No worries! That's what discussion is for. I can easily keep it as a local definition in my init file, or put it on the wiki.

Back then it was decided that such buffer actions are not localized enough and that a local keymap is the better solution for buffer-wide keybindings.

This is a good place to explain why I wrote this in the first place. I bind quite a few extra keys in help-mode-menu (e.g. a command bound to w which copies the symbol the buffer relates to). I was working on adding a few more such bindings in my init file, when I realised that really I was reproducing a lot of what the embark maps do, but with respect to the data in the current help buffer. In order to not duplicate effort (or remember a different set of keybindings), and not have to bind the same actions twice, I just wrote an embark target finder -- now all the actions are availble without me having to bind them separately. And if I ever add more embark actions, those will be reflected too.

You are not truly acting at point. Instead your target finder works like a catch all mechanism which matches when no other more local target matches.

I'm not sure what you mean. It matches only in help buffers, on the thing that the current help buffer describes. It never matches anywhere else. In this way I find it a bit more like the defun finder -- you could be in the middle of the defun, or near the end, or near the beginning, but the target found is the same so long as you are in a defun. This is meant to act like that, but for help buffers.

since everything is a target suddenly.

Again, I'm not sure what you mean. The finder can only every match when called from a help buffer, and in such a buffer, will only match one thing -- the symbol the buffer describes.

(edit to add: this might not be obvious to those who don't use embark this way, but it's at the end of the finder list, so is only really reachable by cycling through shadowed targets -- you're unlikely to hit it unless you want to)

@minad
Copy link
Contributor

minad commented Feb 9, 2023

This is a good place to explain why I wrote this in the first place. I bind quite a few extra keys in help-mode-menu (e.g. a command bound to w which copies the symbol the buffer relates to). I was working on adding a few more such bindings in my init file, when I realised that really I was reproducing a lot of what the embark maps do, but with respect to the data in the current help buffer. In order to not duplicate effort (or remember a different set of keybindings), and not have to bind the same actions twice, I just wrote an embark target finder -- now all the actions are availble without me having to bind them separately. And if I ever add more embark actions, those will be reflected too.

Yes, obviously. The advantage of such non-localized target finders is that you can reuse Embark maps, the same with @oantolin's buffer target.

I'm not sure what you mean. It matches only in help buffers, on the thing that the current help buffer describes. It never matches anywhere else. In this way I find it a bit more like the defun finder -- you could be in the middle of the defun, or near the end, or near the beginning, but the target found is the same so long as you are in a defun. This is meant to act like that, but for help buffers.

I mean that the finder matches at every single point in the buffer - in this sense it is not localized. This is different from other Embark target finders. Personally I think such finders muddy what Embark is about. Usually you act at point, on a symbol at point, on an url at point etc.

I also see some value if Embark tells me that there is nothing specific at point to act on (embark--targets returns nil). This will never happen in help buffers with your target finder and it will never happen in any buffer with @oantolin's buffer finder.

Another criticism I have regarding your target finder is that it won't work in helpful buffers, since the help data is nil. In contrast normal symbol/url finders work. Supporting helpful could of course be fixed easily, but my point is that the target finder you defined here is essentially an extension of the help mode - it is not about targets at point, but really about the mode of the current buffer.

A further oddity of your target finder is that it searches for the symbol and then returns these bounds. This is unlike other target finders, where usually (point) is within or at least at the boundary of the target. These are all small things and one could argue that the advantage of reusing keymaps outweighs the downsides, but I think these small differences in contrast to other target finders sum up. In my opinion it is better to have narrow scope and predictable behavior of features. It is not easy to argue this, but sometimes less is more. In Emacs we often see the mistake that things are added and added without considering if an addition really fits or is really worth it.

(edit to add: this might not be obvious to those who don't use embark this way, but it's at the end of the finder list, so is only really reachable by cycling through shadowed targets -- you're unlikely to hit it unless you want to)

Yes, that's clear. However embark--targets will never be nil, which I consider a downside.

@hmelman
Copy link

hmelman commented Feb 9, 2023

I like this for a couple of reasons but I'm having a hard time generalizing it.

It feels useful here because there's an obvious fallback target in help buffers and actions to use. Since it's at the end of the finder list, it will only be a factor when nothing else matches, i.e., when embark would otherwise not be useful. The reuse is an advantage vs other approaches to the functionality. Looking at other help-like buffers, customize buffers don't have something useful to target and Info buffers could target a node or section name but there isn't much to do with them that would be more efficient. Other special buffers like dired, ibuffer, rmail, shells, list-packages don't really have an obvious target or one that's more useful than the specialized bindings those modes already provide. compilation and grep modes do have the command or search string used to generate the buffer, though I can only think of kill-ring-save as a really useful command to act on them. Maybe there are others I'm missing, but it does seem useful in this case.

The idea of a target being broader than just symbol around point sparked my interest. I have a personal use case for pulling some text from the line point is in, though necessarily around point. I have lists of movies in markdown format following some variable conventions based on the type of list they're in, e.g.,

- [x] 1954 **Rear Window** - Best Director Oscar Nom, NFR 📺️ - Last Seen: 2020-02-01

3. [x] Rear Window (1954)

and I've written some functions to parse these lines for the title and year and font-lock them and commands for: lookup the title with year on IMDb, copy the title to kill-ring, grep the folder to find the title in other lists, etc. My commands function wherever point is on the line. I made a major mode for these files but it derives from markdown-mode so there aren't a lot of free good bindings. Using a target finder here and embark as a prefix seems like a good solution. Obviously this isn't for general use, it's just for me. And it's somewhere between a traditional target like symbol-at-point vs the proposed one here, but the idea that embark could find a target not directly around point seems like an efficiency win.

I found the old discussion for a buffer target and it was a while ago, I think before there was target cycling. I'm sympathetic to purity arguments, but I don't really follow it here. This isn't like adding an ugly argument in an API making it more complext or harder to maintain or extend. I don't see the "value if Embark tells me that there is nothing specific at point to act on". Usually that means point is not where I thought it was and I need to move it. Maybe that's the case here, but I think this target is way more useful than that (I can notice the target is wrong and move point just like any other time point isn't where I think it is, which I don't think I run into often). I guess there's a downside of one more target to cycle through, but this is a far less general case than that of "this-buffer".

IMHO the utility of this finder and its specificity outweigh these downsides . People can reasonably differ on this but I think that's what configuration is for. I think this would be useful and I like it as an example for people to think of other non-obvious targets. It could go in the wiki but I think it would be a little lost there (it's harder to follow updates there). I like this as an example that targets should be obvious and useful and not just "around point" which embark's collection functionality already muddies. I have pause because I can't think of too many other examples, but this one genuinely seems neat.

@oantolin
Copy link
Owner

oantolin commented Feb 9, 2023

I could go either way on this one. I don't share @minad's worry about target finders that match anywhere in a buffer, because of my experience with the buffer target finder I put on the wiki. I still have it in my personal configuration, and while I don't use it much I've never felt it go in the way. Any information you can glean from having no targets found I instead glean from having the buffer be the first target, it's almost the same (the difference is that if there are no targets found, you get a message and embark-act exits, with the buffer target finder you need an extra C-g, which I don't mind). And having said that I don't find the information that there are no targets at point very useful, maybe occasionally it tells me I have unbalaneced parenthesis but not often.

Now, I wouldn't add the buffer target finder to Embark's default configuration, but this one feels different. I think that since the help buffer is typically something you just read and don't have a complex interaction with, it is "small context" despite its size, and to me it does not feel too large as a target representing the thing you got help on. I haven't tried this target finder yet so I don't have a feeling for how complete its heuristic type guessing is. Maybe it would be better without bounds, though (my buffer target finder does not report bounds, and the lack of highlighting is a good cue indicating the target is not something very small around point).

@Hugo-Heagren
Copy link
Author

Thanks for the good feedback everyone! I have to say it's nice to see a package/community where things like are discussed in such depth. That isn't always true with other packages.

Maybe it would be better without bounds, though (my buffer target finder does not report bounds, and the lack of highlighting is a good cue indicating the target is not something very small around point).

The idea of adding bounds was to indicate to the user what they're acting on -- there might be lots of symbols in the help buffer, and running embark might also pick up the one at point.

@minad
Copy link
Contributor

minad commented Feb 9, 2023

@oantolin

I still have it in my personal configuration, and while I don't use it much I've never felt it go in the way.

That's not exactly an argument in favor, if we add target finders which just sit there at the end of the list but we never use them.

I also wonder about the convenience of this finder. Does it save us some key strokes? You can always press M-< to move to the beginning of the help buffer (where we will find the symbol) and then press C-. to act.

And having said that I don't find the information that there are no targets at point very useful, maybe occasionally it tells me I have unbalaneced parenthesis but not often.

It is useful if you write dwim commands based on embark-dwim which perform something else if Embark doesn't match. But I have to check my configuration, maybe Embark comes last anyway since it already matches too often. ;)

@oantolin
Copy link
Owner

oantolin commented Feb 9, 2023

maybe Embark already comes last anyway since it already matches too often. ;)

😄

@minad
Copy link
Contributor

minad commented Feb 9, 2023

@Hugo-Heagren

The idea of adding bounds was to indicate to the user what they're acting on -- there might be lots of symbols in the help buffer, and running embark might also pick up the one at point.

FWIW I would prefer if the first symbol is highlighted in the buffer to make it clear what is going on.

I may give up my resistance against such non-localized/universal targets, except for @oantolin's buffer finder.

As a courtesy for me it would be great if you could make the finder work in helpful buffers too. Or maybe that was the courtesy right there, that this target finder does nothing in my setup. Haha.

I wonder if there are other such non-localized finders. @hmelman if I read you correctly, you didn't find any? Usually I consider it a validation of a design if we can come up with more than a single example. Vice versa single special cases are better rejected.

@hmelman
Copy link

hmelman commented Feb 10, 2023

I would also want this finder to work in helpful buffers.

@minad I didn't come up with much. I thought about modes built into emacs, not 3rd party packages. Mostly I thought about special- modes. Of those I think compilation- and grep- modes might have some utility if there are interesting things to do with the commands/strings that initiated them. Combining grep strings with internet search functions could be useful. Or maybe in a compilation buffer, an error message (on the same line or the nearest one?) could be a target and you could search for it on google, stackoverflow or Info, or paste it into notes.

Most things derived from text- or prog- mode didn't seem reasonable as with the this-buffer idea, but I wonder about outline modes, where the closest constraining header is the target (the text of the header as opposed to the whole section). Particularly with targets that aren't under point, marking the target is useful as a goto command. I'm not an edebug user, but maybe something there?

Maybe help buffers are just special because they have a single thing they are about? That also applies to dictionary-mode and embark first targeted an id and then an expression, which was kind of useless. Targeting the word looked up would be useful and similar to the help case. This could apply to the title of an eww buffer. So maybe these make it viable?

@minad
Copy link
Contributor

minad commented Feb 11, 2023

This could apply to the title of an eww buffer. So maybe these make it viable?

For eww buffers it would probably be the URL of the buffer. I think that could actually be useful. I often want to obtain the current URL, but there is already eww-copy-page-url.

See also what I wrote here:

I also wonder about the convenience of this finder. Does it save us some key strokes? You can always press M-< to move to the beginning of the help buffer (where we will find the symbol) and then press C-. to act.

So overall both such finders would yield small benefits only, but this can probably be said about many Embark actions. Embark is also about discoverability, even if actions don't become significantly more accessible. In this case it is really just the question if we want non-localized targets, which I consider inconsistent with existing Embark target finders (thinking about the point not being on the target and thinking about the bounds used for highlighting), but maybe it is a reasonable generalization.

@hmelman
Copy link

hmelman commented Feb 11, 2023

For eww buffers it would probably be the URL of the buffer. I think that could actually be useful. I often want to obtain the current URL, but there is already eww-copy-page-url.

Right, there's already a command for it, but the title is shown in a header which M-< doesn't bring you to, so I thought it was more useful as a fallback target.

In this case it is really just the question if we want non-localized targets, which I consider inconsistent with existing Embark target finders (thinking about the point not being on the target and thinking about the bounds used for highlighting), but maybe it is a reasonable generalization.

I agree, although I think I like the idea a little more than you. I'd like it if the idea got a little more exposure, maybe others could think of better uses. It could go in the wiki, but I'm not sure many would see it. If it was provisionally in embark that would be good, or maybe written up in the README as an example of writing a (weird) target finder. I do think this help(ful) target is good enough to include, even if the concept isn't generalized and that would expose it more. I'd understand if you don't agree.

@minad
Copy link
Contributor

minad commented Feb 11, 2023

@hmelman

I agree, although I think I like the idea a little more than you. I'd like it if the idea got a little more exposure, maybe others could think of better uses. It could go in the wiki, but I'm not sure many would see it. If it was provisionally in embark that would be good, or maybe written up in the README as an example of writing a (weird) target finder. I do think this help(ful) target is good enough to include, even if the concept isn't generalized and that would expose it more. I'd understand if you don't agree.

Well, I think Embark is on the road to stabilization with a version 1.0 in sight. From my pov, this means that "weird" features should not get in. I mean, even if you like to experiment more, there seems to be agreement that the feature is unconventional and "weird". The wiki is the best place for provisional features. Embark is not my package but I care about it, and provisional features increase maintenance, lead to bugs and bug reports. Your perspective is different - I've seen you often asking for something to be added, which from the perspective of a user is understandable. You argue that more exposure is needed to figure out more ideas. Maybe that's the case, but I think it is also reasonable and prudent to ask right away for more such patterns to be found, so see if the generalization is in fact generally useful.

In earlier versions, Embark did neither have target highlighting nor target cycling. When we added this, I believe, that the user and beginner friendliness of Embark was greatly improved. Now we are discussing the addition of finders which do not naturally have a region which can be highlighted. I've argued before that sometimes less is more, see
#595 (comment).

If we add a feature we should ask if it helps many users and not only the experts. For the experts (let's say everyone who can write a finder) it is not a big deal to copy something from the Embark wiki. For beginners that's not the case, but they probably prefer if the feature set overall is consistent and easy to understand. As another example - I am often asked to add customization variables for special edge cases to my packages. While such an addition may make configuration of such special cases easier, it also increases the "customization surface" which a new user has to understand. For the expert (the only one who cares about this special case, or is even able to discover it), again it is no problem to copy a solution from the wiki or write an advice.

@hmelman
Copy link

hmelman commented Feb 11, 2023

Yeah, it's the nature of things that users want more features and (good) developers want less. It's rare for a user to argue for less because features they don't use usually aren't a burden to them.

I came to Embark for the collect functionality and it took a while to get the non-minibuffer utility of it. In the original "this-buffer" target discussion I was doubtful of the usefulness. Now Emacs 28 has a C-x x keymap for buffer commands which I think validates the usefulness but means embark doesn't need it. ;)

It's of course true that users that can write a target finder can write this target finder, but they have to think of it to do so. I found the idea novel and potentially useful for my own config, but hadn't thought of it. I found the idea intriguing enough to try to generalize it and came up with only a few potential targets, maybe others could find more.

The README's section "Defining actions for new categories of targets" I think is a good place to discuss what makes a good target and what doesn't. If targets really should contain point I think it should say so there, or mention the issues that might arise if they don't. Maybe it includes this finder as an example, or points to the wiki or this discussion for more info.

@oantolin
Copy link
Owner

oantolin commented Feb 11, 2023

Maybe help buffers are just special because they have a single thing they are about?

Yes, that's what I was trying to get at @hmelman; you said it much better than I did.

That also applies to dictionary-mode and embark first targeted an id and then an expression, which was kind of useless.

Well, personally, I like that the word (well, identifier) at point is targeted first in that case! I use this sometimes to look up in dictionary some word used in the definition of the word I looked up first. Having the looked-up word as a later target, however would be useful.

But probably the mere fact that we disagree about whether the word at point should have precedence over the looked-up word means that this is best left to personal configuration...

This could apply to the title of an eww buffer. So maybe these make it viable?

I think that for the title of a eww buffer the only sensible action action is to save on the kill-ring, so probably thatś best added as a new command in eww-mode. I don't really like adding targets for which there is only one sensible action.

@hmelman
Copy link

hmelman commented Feb 11, 2023

That also applies to dictionary-mode and embark first targeted an id and then an expression, which was kind of useless.

Well, personally, I like that the word (well, identifier) at point is targeted first in that case! I use this sometimes to look up in dictionary some word used in the definition of the word I looked up first. Having the looked-up word as a later target, however would be useful.

I phrased it badly, targeting an id first is good, the expression target didn't do much for me. (That it came up could just be in my config, but I don't think my config does much). To be clear, I think in a dictionary buffer I'd like the word-at-point to be the first target and the word of the lookup to be the second (or last if there are better ones in between). I think that mirrors the help buffer case.

This could apply to the title of an eww buffer. So maybe these make it viable?

I think that for the title of a eww buffer the only sensible action action is to save on the kill-ring, so probably thatś best added as a new command in eww-mode. I don't really like adding targets for which there is only one sensible action.

Agreed. I could see other commands that make a link of out it (say an org-link) but I think that would be via a custom command rather than a builtin one, so if that's happening, someone could make a custom embark config too. Is this basically the case for all read-only text that isn't an emacs type like a symbol, buffer or file with lots of commands acting on them? There just isn't much to do with it other than put it on the kill-ring or do an Internet search.

@minad
Copy link
Contributor

minad commented Feb 11, 2023

@hmelman

Yeah, it's the nature of things that users want more features and (good) developers want less. It's rare for a user to argue for less because features they don't use usually aren't a burden to them.

But the thing is this - Emacs blurs this boundary. That's what makes Emacs so great, since you can make it yours. This means if you consider it good practice to avoid stuffing too much functionality into something, the users should also be more modest in their demands. ;)

I must admit I think the community is wasting a lot of effort with personal massively tweaked configurations. I think the shift should go more in the direction of package development, where everything self contained is packaged. This is why I find @oantolin's configuration so great (and my own, which only imitates that style :-P). Since Omar has everything non trivial implemented as a small library which could possibly be published officially. It is a playground for bootstrapping.

The README's section "Defining actions for new categories of targets" I think is a good place to discuss what makes a good target and what doesn't. If targets really should contain point I think it should say so there, or mention the issues that might arise if they don't. Maybe it includes this finder as an example, or points to the wiki or this discussion for more info.

Indeed, the best practices should be documented there. However this is what this discussion is about - it is not yet clear what makes up a good target and what does not.

@oantolin

I think that for the title of a eww buffer the only sensible action action is to save on the kill-ring, so probably thatś best added as a new command in eww-mode. I don't really like adding targets for which there is only one sensible action.

Therefore I think the URL target would be more useful. However it also suffers from the problem that the target cannot be highlighted and that the target has nothing to do with (point). Therefore I also consider it dubious.

@hmelman
Copy link

hmelman commented Feb 15, 2023

So I got around to trying this and I think I like it. Recall I have markdown files of lists of movies formatted in a few different ways such as:

- [x] 1954 **Rear Window** - Best Director Oscar Nom, NFR 📺️ - Last Seen: 2020-02-01

3. [x] Rear Window (1954)

I have a major-mode that can parse these lines (rx is nice for this) and font-lock them and some commands that operate on the titles and/or years. In general they all act like dired, operating on a line doing whatever is necessary, toggling an emoji or pulling out the title to grep it or search for the "title (year)" on IMDb. Unlike dired I do edit these files as plain markdown. I had tried including an overlay keymap hrm-film-title-map when point was on a title, but the single key bindings got in the way of normal editing. I had settled on bindings using a hyper key and it was working okay (I configure my mac's right command key as hyper though I'm not completely sold on it) .

I found doing the following works quite well with embark. I get highlighting of the title as a target (was that supposed to be an issue?) and since the commands already move to the title if needed, they "just work". I have one command that works on years and will try adding a second target for years. My embark-act is on super so using s-; g instead of H-g isn't bad. I'm not sure there's huge value in this case (since the command already do the parsing, but I could change them to take a title as a string argument.

(defvar hrm-film-title-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "f") #'hrm-film-title-imdb)
    (define-key map (kbd "r") #'hrm-film-title-mark)
    (define-key map (kbd "b") #'hrm-film-title-bold)
    (define-key map (kbd "g") #'hrm-film-title-rg)
    (define-key map (kbd "s") #'hrm-film-title-rg-seen)
    (define-key map (kbd "t") #'hrm-film-title-rg-tivo)
    (define-key map (kbd "T") #'hrm-film-title-titlecase)
    (define-key map (kbd "v") #'hrm-film-toggle-tivo)
    map)
  "Keymap automatically activated on titles.")

(defun hrm-film-embark-target ()
  "Return the title on the current line as an embark target."
  (when (derived-mode-p 'hrm-film-mode)
    (seq-let (tbeg tend _ybeg _yend) (hrm-film-title-bounds)
      `(film ,(buffer-substring-no-properties tbeg tend) ,tbeg . ,tend))))

(add-to-list 'embark-target-finders 'hrm-film-embark-target)

(add-to-list 'embark-keymap-alist '(film hrm-film-title-map))

I haven't played with the new treesitter stuff, but I wonder if its better parsing would enable some interesting use models with non-local targets in such modes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants