search

minad / consult

:mag: consult.el - Consulting completing-read
GNU General Public License v3.0
1.14k stars 99 forks source link

Add consult-register-load/save #174

Closed minad closed 3 years ago

minad commented 3 years ago

@oantolin I saw you commenting on reddit about registers and it reminded me again that maybe something should be done about the register access functions. I quickly imported your utilities from your https://github.com/oantolin/emacs-config/blob/208755c715284f0457e852e777a38a463b1aefcb/my-lisp/text-extras.el. The functions are all in the consult-register namespace and could possibly go to a separate library. What do you think about having such functions here? Maybe it makes sense to not have these functions here since they are not related to completing read and are simple enough to keep around in the init.el. The load function can barely be criticized, I think it should be upstream in this form. The save function is more questionable since, as you said, it is rather ad-hoc. But arguably, saving strings, numbers and points are the most important use-cases for registers during editing.

minad commented 3 years ago

See also the original discussion about consult-register in #77.

minad commented 3 years ago

As a side note - in case one would like to use a single keybindings for loading/storing, there is a trick:

(condition-case nil
    (cons 'load (register-read-with-preview "Load register: "))
  (error
    (cons 'save (register-read-with-preview "Save register: "))))

If the command is bound to M-#, then M-# a will load register a, and M-# M-# a will save to register a, since register-read-with-preview only accepts characterp keys.

minad commented 3 years ago

Ping @protesilaos and @hmelman for opinions.

@protesilaos Didn't you have some prot/register library at some point? I saw something in your video.

protesilaos commented 3 years ago

@protesilaos Didn't you have some prot/register library at some point? I saw something in your video.

Yes, I recently removed them because I only used them once or twice in the span of several months. Though I still use registers all the time. Will read was is here and act accordingly. Any improvements on working with registers are most welcome.

hmelman commented 3 years ago

Since I load from melpa it's hard for me to play with this branch, but reading the code I think I'm good with the idea; particularly since the single keybinding hack sounds intriguing.

The docstring for consult-register needs some cleanup. "Built-in" is duplicated. For the recommendation, I'd be more explicit that the other commands will use fewer keystrokes to enter a register name. I don't love the term "load" since I confused it with "loading to" or "loading from" a register. If you're going with the hardware register terminology then use both "load" and "store" (not "save"); but while Emacs doesn't have a general term for using a register, I think I like "use" better. How about this:

"Use register by jumping to the location or inserting the stored text.

 This command is useful to search the register contents. For faster
 entry of a register name use the register commands
 `consult-register-load', `consult-register-save' or the built-in 
 register access functions. 

This command supports narrowing, see  `consult-register-narrow'. 
Marker positions are previewed. See `jump-to-register' and
`insert-register' for the meaning of ARG."

Also, if the commands really are "do what I mean" I'm a fan of following the convention of including "-dwim" in the command name.

minad commented 3 years ago

@hmelman

I don't love the term "load" since I confused it with "loading to" or "loading from" a register. If you're going with the hardware register terminology then use both "load" and "store" (not "save"); but while Emacs doesn't have a general term for using a register, I think I like "use" better. How about this:

Regarding the name, I am either going with consult-register-load/store or consult-register-use/store. But this is not yet settled.

The docstring for consult-register needs some cleanup.

Agree.

particularly since the single keybinding hack sounds intriguing.

I haven't implement this here. Would you like to see something like this instead of two commands? Something like a consult-register-access command which either loads or stores when you press the key binding of consult-register-access again. I am not sure about it. It was just an idea.

hmelman commented 3 years ago

Regarding the name, I am either going with consult-register-load/store or consult-register-use/store. But this is not yet settled.

FWIW the Emacs manual uses "Save" in all the section headings and often the term "store" in the text. Command names use neither sticking with the -to-register pattern, but several of the docstrings use "store" and some use "restore" (which is interesting). I think my vote is either load/store sticking with hardware terms or use/save as colloquial english but I don't love any of the options so far.

I haven't implement this here. Would you like to see something like this instead of two commands? Something like a consult-register-access command which either loads or stores when you press the key binding of consult-register-access again. I am not sure about it. It was just an idea.

I'm not sure. As previously discussed the value of these commands over the builtin ones is a bit limited. Combining them into one command seems like value add and I'd at least play with it. If you don't actually implement the command, then at least document the hack in the README or manual. I wish the functionality of the save side (currently just text, number or point) was more symmetrical with the use side (additionally rectangles, window/frame configs, filenames) but I understand the limitation. Maybe it's a reason to just doc it and not implement it.

I think registers are a little known/used feature of emacs and consult making them more attractive at all is probably good advertising for them. 😄

oantolin commented 3 years ago

I wish the functionality of the save side (currently just text, number or point) was more symmetrical with the use side (additionally rectangles, window/frame configs, filenames) but I understand the limitation.

Oh, did you remove saving frame configurations from my function, @minad?

minad commented 3 years ago

I added a magic consult-register-access command. It either loads or stores. By pressing the access key again you can force it to overwrite. Maybe this is too much. It uses the idea from here https://github.com/minad/consult/pull/174#issuecomment-765395039.

@hmelman

I think my vote is either load/store sticking with hardware terms or use/save as colloquial english but I don't love any of the options so far.

Yes, this is what I proposed - consult-register-load, consult-register-store. It makes sense to have a common consult-register prefix.

@oantolin

Oh, did you remove saving frame configurations from my function, @minad?

Not that I know of. I think I copied them more or less verbatim and then applied some minor tweaks. Maybe I used an old version?

minad commented 3 years ago

@hmelman

I wish the functionality of the save side (currently just text, number or point) was more symmetrical with the use side (additionally rectangles, window/frame configs, filenames) but I understand the limitation.

This is hard to do in a good way in a single command. For the lesser used functionality like rectangles, windows, frames it may be better to just use the builtins. But I could also add some magic via the same trick as the consult-register-access function I just proposed.

hmelman commented 3 years ago

I think my vote is either load/store sticking with hardware terms or use/save as colloquial english but I don't love any of the options so far.

Yes, this is what I proposed - consult-register-load, consult-register-store. It makes sense to have a common consult-register prefix.

I was commenting on your "or consult-register-use/store" comment. My thought was if you go with use, also go with save. But I think we're mostly in agreement.

Oh, did you remove saving frame configurations from my function, @minad?

Not that I know of. I think I copied them more or less verbatim and then applied some minor tweaks. Maybe I used an old version?

I guess I missed it, just reading the code I saw point-to-register not realizing it saved framesets with an arg.

oantolin commented 3 years ago

Also, the function does save rectangles too, if the region is active and is a rectangle. So it's not that asymmetric, I'd say. @hmelman

hmelman commented 3 years ago

Also, the function does save rectangles too, if the region is active and is a rectangle. So it's not that asymmetric, I'd say.

In that case I agree and think the function deserves the -dwim prefix 😄

minad commented 3 years ago

I have another idea. What about extending the consult-register-narrow variable and also adding a store function? Then I could do the following:

M-# a -> load register a if it exists
M-# a -> store register a if it does not exist (dwim)
M-# M-# a -> overwrite register a (dwim)
M-# M-f a -> store something in register a via the store function specified by the f narrow key, e.g. frameset

This way it would be extensible.

hmelman commented 3 years ago

I like the expanded functionality but don't love the mechanism which IIUC wouldn't expose these keybindings to which-key or other help commands.

minad commented 3 years ago

I like the expanded functionality but don't love the mechanism which IIUC wouldn't expose these keybindings to which-key or other help commands.

That's right. It could be rewritten using a keymap or I could show a help for M-?. I don't want to over complicate things. For me the main benefit of this would be that I immediately see the free/used registers. And the register access would be fast since it would only require two or three key presses to store in the dwim fashion, while going through a single entry point. Maybe it wouldn't give significant benefits over the load/store functions by @oantolin.

@oantolin What do you think?

minad commented 3 years ago

@hmelman I could put the help directly in the register preview buffer at the end. Furthermore I would probably implement this a bit differently such that the register preview window comes up after the configured register preview delay. The approach can be described as registers+actions, where the actions are put onto the non-character/non-register keys.

oantolin commented 3 years ago

I'm not a big fan of being able to select extra types of storage with extra keys. We already have the built-in C-x r prefix for that. And what is actually left to store? Is it just file names and window configurations?

I don't think there is a built-in command for filenames, so maybe it's not a frequent operation, maybe it's just something people do in their init.el. That makes sense to me, actually, like single letter bookmarks for files. Interactively, instead of storing a filename, I think I'd just store a window configuration that includes a window visiting the file. Do we really need a way to store file names in these commands?

As for window configurations, maybe it would be nice to be able to store them instead of frame configurations, or in addition to them. For me these are almost the same since I rarely have more than one frame. In the function @minad started with I went with frame configurations out of laziness, just because the built-in point-to-register gave them to me for free, but maybe it would be better for C-u to mean window configuration and C-u C-u to mean frame configuration.

minad commented 3 years ago

I'm not a big fan of being able to select extra types of storage with extra keys. We already have the built-in C-x r prefix for that. And what is actually left to store? Is it just file names and window configurations?

Actually, in Emacs 27 or later I think you can store arbitrary things in the registers. You only have to define some kind of register cl-struct.

So you also don't like the possibility to go through a single command for loading/storing/overwriting? For me the difference is that I would see the registers earlier than when going via C-x r, e.g., C-x r f a vs M-# M-f a. Or with your command this would be C-u C-u M-'. But it is just an idea, I think I won't add it here if it is controversial.

As for window configurations, maybe it would be nice to be able to store them instead of frame configurations, or in addition to them. For me these are almost the same since I rarely have more than one frame. In the function @minad started with I went with frame configurations out of laziness, just because the built-in point-to-register gave them to me for free, but maybe it would be better for C-u to mean window configuration and C-u C-u to mean frame configuration.

Agree, I also use mostly a single frame.

hmelman commented 3 years ago

In the function @minad started with I went with frame configurations out of laziness, just because the built-in point-to-register gave them to me for free, but maybe it would be better for C-u to mean window configuration and C-u C-u to mean frame configuration.

I'd be fine with this and ignoring filenames. I typically use two frames and sometimes a dedicated 3rd (info, gnus). I haven't really saved window or frame configs but it's sometime I've considered playing with.

hmelman commented 3 years ago

But it is just an idea, I think I won't add it here if it is controversial.

Have you considered a consult-experimental package to try out such things and get some feedback without committing to supporting them if they don't work out?

minad commented 3 years ago

To save (and persist) window configs I am using https://github.com/minad/bookmark-view btw. This is similar to burly, which you may have seen, but as simple as possible.

minad commented 3 years ago

Have you considered a consult-experimental package to try out such things and get some feedback without committing to supporting them if they don't work out?

Yes, but I don't like the idea. If there is something controversial/experimental it should just not get in here. I will maintain such commands in my own init.el, if it seems that for some weird reason I am the only one who likes the commands :laughing:

protesilaos commented 3 years ago

oantolin: As for window configurations, maybe it would be nice to be able to store them instead of frame configurations, or in addition to them. For me these are almost the same since I rarely have more than one frame.

This is actually something I wanted to investigate, but... In my experience window-configuration-to-register cannot be revisited/recreated after you restart Emacs. Whereas that of frameset-to-register can. Note that I still need to double-check this.

minad: To save (and persist) window configs I am using https://github.com/minad/bookmark-view btw. This is similar to burly, which you may have seen, but as simple as possible.

Then maybe my previous point stands. I need to check that package, by the way.

oantolin: I'm not a big fan of being able to select extra types of storage with extra keys. We already have the built-in C-x r prefix for that. And what is actually left to store? Is it just file names and window configurations?

I always prefer dwim-style commands over extra keys.

(defun consult-register-store (reg &optional arg)
  "Store what I mean in a REG.
With an active region, store or append (with ARG) the contents, optionally
deleting the region (with a negative argument). With a numeric prefix, store the
number. With ARG store the frame configuration. Otherwise, store the
point."

The doc is a bit terse and the hour is growing late. To unpack it in order to be sure I got this right:

Is this correct? If so, it looks good.

defun consult-register-load (reg &optional arg)
  "Do what I mean with a REG.
For a window configuration, restore it. For a number or text, insert it. For a
location, jump to it. See `jump-to-register' and `insert-register' for the
meaning of ARG."

Fine by me. I assume it also covers framesets, since jump-to-register does.

minad commented 3 years ago

@protesilaos

Then maybe my previous point stands. I need to check that package, by the way.

Yes, please do. For me it mostly seems to work - the key is that bookmarks can be saved/restored. So you only have to walk over all windows and persist the bookmarks too. This way you basically get persistent window configurations for free.

I always prefer dwim-style commands over extra keys.

The idea we discussed here was to provide a single consult-register-access command which is mostly dwim but supports additional "actions" via extra keys. If you would bind that command to M-# the following would work:

M-# a -> load register a if it exists
M-# a -> store register a if it does not exist (dwim, either point or region or framesets depending on C-u)
M-# M-# a -> store/overwrite register a (dwim, either point or region)
M-# M-f a -> store something in register a via the store function specified by the f narrow key, e.g. frameset

Then there could be additional modification of the behavior via C-u prefixes. The question is if such a do-it-all consult-register-access command would be better than having two separate more limited consult-register-load/store commands.

In contrast with consult-register-load/store bound to M-# and M-' you can do the following:

M-# a -> load register a if it exists
M-' a -> store/overwrite register a (dwim, either point or region or frameset depending on C-u)

The doc is a bit terse and the hour is growing late. To unpack it in order to be sure I got this right:

I think you got the unpacking right. The docstrings should be improved before pulling it in.

protesilaos commented 3 years ago

minad: Then there could be additional modification of the behavior via C-u prefixes. The question is if such a do-it-all consult-register-access command would be better than having two separate more limited consult-register-load/store commands.

Ah yes, I see. I would honestly need to see it in practice to make an assessment. Now I feel that separate load/store commands would be easier to understand. And it follows the default scheme of copy/insert and store/jump.

minad commented 3 years ago

Ah yes, I see. I would honestly need to see it in practice to make an assessment. Now I feel that separate load/store commands would be easier to understand. And it follows the default scheme of copy/insert and store/jump.

I agree with this copy/insert - store/jump scheme. In practice you will always know if you want to store or load before calling a register command, thus two commands are appropriate. The only point of consult-register-access would be that you save a keybinding since everything goes through M-#, but this is not really justifiable.

However the idea of using M-f, M-w to store other values like framesets or windows can also work if we are going with separate consult-register-load/store commands. For example, you could type M-' M-w a to store the window configuration in register a. This wouldn't be significantly better than calling the already existing C-x r w a as @oantolin notes. The main point would be that all register store commands could go through the unified prefix M-', instead of using C-x r ... to store some values and M-' for storing points, regions and numbers in a dwim fashion.

minad commented 3 years ago

I think it makes sense to explore the action idea. I would have two different commands consult-register-load and consult-register-store. The load command will either jump or insert. The store command will be mostly dwim depending on the current status. Instead of encoding the alternative actions as prefix arguments the actions could be shown in the register preview window and encoded as M-keys.

EDIT: I pushed two variants now 1. consult-register-store-prefix and 2. consult-register-store-action. 1. is basically @oantolin's variant with C-u prefix arguments to chose between different behaviors. 2. is the variant where the operation can be chosen by some M-key instead. If someone is interested in trying it out, I would appreciate some feedback.

oantolin commented 3 years ago

Well, the action variant already sounds better than my original function, because the sequences like M-w or M-i are much more mnemonic than some arbitrary number of C-us! Plus, as you said you get to see the register preview sooner, because the action selectors come after the command, unlike the C-us which come before.

I think you could probably get rid of the prefix variant. It sounds like it has worse UI all around, and that the only features it has that the action version lacks is the option to delete the region once you store it. That extra functionality doesn't really need to be in the command: if it takes an extra keypress to do the deletion anyway, you might as well just C-w after the register store command.

When I said earlier that I didn't think that we should recreate the C-x r prefix I meant specifically using letter with the same meaning the have there, mostly to indicate the type stored in the register. But I really like p, a and i for prepend, append and increment (which are actions, not datatypes!). And yes, here f and w do recreate their functions in C-x r, but the p, a and i justify the command to my point of view, and then the f and w just complete the functionality.

minad commented 3 years ago

Yes, for me it is just a slightly more convenient and helpful encoding in comparison to the C-u prefix encoding of your command. Regarding the deletion, I still use a prefix for that. But you are probably right that this is useless functionality since you can always press C-w afterwards. We would only win something if we bind a deleting variant to a separate key. But then the whole point of this command is that we want to save keys. And good keys like M-# are rare.

Edit: To clarify, the plan is to add only one of the two variants to consult, probably the action variant with the better UI.

oantolin commented 3 years ago

Well, there is a tiny difference, since I believe the built-in register commands delete the region, rather than killing it, so it doesn't "pollute" the kill ring. To be honest, my attitude to the kill ring is "the more the merrier", so I think C-w after the register store command is better: there's a chance I reuse the register and realize I still need the text and it would be nice to have a backup in the kill-ring. (The "kill-ring pollution" complaints I sometimes see sound like they must come from people who don't use consult-yank, helm-yank, browse-kill-ring, etc.)

oantolin commented 3 years ago

Edit: To clarify, the plan is to add only one of the two variants to consult, probably the action variant with the better UI.

I understood that all along. I was saying you can already declare the action version the winner. I seriously doubt anyone would prefer the prefix version.

Both versions have in common the core value proposition: for the "basics", storing text and positions, you need no extra keys, just invoke the command. And then where the differ, I think the action version is the same or clearly a little better in every aspect.

minad commented 3 years ago

Well, there is a tiny difference, since I believe the built-in register commands delete the region, rather than killing it, so it doesn't "pollute" the kill ring.

We can keep the C-u prefix for the purists.

minad commented 3 years ago

Merged in https://github.com/minad/consult/commit/1f9c35b090b1e12fe67589af3262f6ded14452a6