Target highlight

[minad]

I hope you don't mind me throwing another PR at you. I am a big fan of the universal Embark at point functionality! Addresses #269 partially. Some of the finders must still be overhauled, but this is also due to #278/#92.

EDIT: I think all the at point target finders work properly with highlighting now. I also deprioritized and broadened the sexp and defun finders.

[oantolin]

I hope you don't mind me throwing another PR at you. I am a big fan of the universal Embark at point functionality!

Mind? Are you kidding? On the contrary, I'm very grateful! This is great stuff.

[minad]

Mind? Are you kidding? On the contrary, I'm very grateful! This is great stuff.

Thanks ❤️

[oantolin]

Expanding where the sexp and defun targets match broke all the actions. :(

[minad]

Expanding where the sexp and defun targets match broke all the actions. :(

Uh right, the primitive movement to the beginning does not work anymore. What about using the target finder itself to find the boundaries, now that the finder returns them?

[minad]

The defun map should also not inhert the expression map anymore I guess. Or alternatively the map should still inherit but the expression finder should not find a target when the defun finder matches (EDIT: You already did this, I didn't notice when I wrote this comment). I have to think a bit about this.

[oantolin]

If we expose the bounds, say put them in an embark--target-bounds variable, we could write mark, kill and indent actions that work for all at-point targets at once.

[minad]

If we expose the bounds, say put them in an embark--target-bounds variable, we could write mark, kill and indent actions that work for all at-point targets at once.

Sounds like a good and simple idea. Otherwise the action would not know which finder to call. And since we have the information readily available, this sounds about right.

[oantolin]

I'll add the variable. I don't like adding variables, but it's probably worth it for this.

[minad]

I'll add the variable. I don't like adding variables, but it's probably worth it for this.

Me too. But I think this sounds right. This may even bring us a step closer to unifying the region actions too.

[minad]

The embark--target-bounds variable is basically the means to inject boundaries into an action. We have been lacking this. It is not pretty, but there is probably no reasonable alternative.

If there is the "r" interactive specification, we could pass the region arguments directly. But I am not fond of complicating the Embark calling convention.

[oantolin]

OK, I added the variable and fixed the sexp actions. 478ffcc

Now we just need to decide what the right level of generality is for those actions. I guess I could move them to the general map now. What do you think?

[oantolin]

I mean, not raise, but indent, kill and mark could use the region functions and operate on any target.

[minad]

Now we just need to decide what the right level of generality is for those actions. I guess I could move them to the general map now. What do you think?

Right, now the indent/kill/mark are more general. But having it in the general map is not a good idea because it does not work in the minibuffer.

Now I would like to have some automatic keymap derivation mechanism again. Hmm. Maybe introduce an embark-bounds-map which contains these actions which require target bounds. When acting at point and if the action keymap derives from the general map also merge in the embark-bounds-map automatically, such that the inheritance is action-keymap>bounds-map>general-map.

Then the question is if the embark-bounds-map could ultimately be the same as the embark-region-map.

[minad]

Okay, it should be like I described above, but embark-region-map/=embark-bounds-map. But the region finder should also return the boundaries of the region, and the region-map should inherit from the new bounds-map, since the bounds-map is more general.

[oantolin]

Then the question is if the embark-bounds-map could ultimately be the same as the embark-region-map.

I was thinking that maybe if the action name ends in -region, embark--act could arrange for the target bounds to be set as as the region. I mean, it could also always set the region to the bounds, but people probably don't want Embark messing with the region all the time.

[minad]

I was thinking that maybe if the action name ends in -region, embark--act could arrange for the target bounds to be set as as the region. I mean, it could also always set the region to the bounds, but people probably don't want Embark messing with the region all the time.

I've also considered adjusting the region, but it does not seem like a good idea. I also don't like the magic of checking if the action name ends with -region.

We could also call the bounds map embark-at-point-map.

[oantolin]

• embark-at-point-map sounds nicer.
• I don't think the region map should inherit from the at-point map: the region map contains unwrapped -region commands for which this new map would contain a wrapper.

[oantolin]

You know, we could also just leave things as they are for a bit. I don't think I've ever really wanted to indent the region consisting of the file name at point. 😉 Maybe only kill would really be useful to generalize.

[minad]

I don't think the region map should inherit from the at-point map: the region map contains unwrapped -region commands for which this new map would contain a wrapper.

I agree. But it would work in principle and it would unify things. At the same time it wouldn't make much sense, since marking the region again is nonsense 😆

You know, we could also just leave things as they are for a bit. I don't think I've ever really wanted to indent the region consisting of the file name at point. wink Maybe only kill would really be useful to generalize.

Okay, I agree. Better than reintroducing the auto inheritance mess again. We will regret it later.

[oantolin]

By the way, the commands in the region map that also prompt at the minibuffer, and which therefore wouldn't work with a string target, seem to be only three: shell-command-on-region, align-regexp, and write-region.