sminez / ad

an adaptable text editor
https://crates.io/crates/ad-editor
MIT License
243 stars 9 forks source link

Question: Thinking about plumbing rules and auto-expanding dot if it's not a range #38

Closed izuzak closed 2 weeks ago

izuzak commented 3 weeks ago

Hello! 👋 I'm again opening a question (instead of a feature request or bug report) since I'm not sure if and what should be improved here. :v:

Context: I work in on a codebase which has lots of foo/bar references in comments which refer to github repository names, e.g. sminez/ad refers to https://github.com/sminez/ad, and also lots of foo/bar#123 references which refer to a specific issue or PR in a github repository, e.g. sminez/ad#12 refers to issue https://github.com/sminez/ad/issues/123.

I wanted to create plumbing rules to allow me to open such references as link in the browser when I press Enter. So, I created these rules:

data matches ([a-zA-Z0-9\-_]+/[a-zA-Z0-9\-_]+)#([0-9]+)
plumb start open https://github.com/$1/issues/$2

data matches [a-zA-Z0-9\-_]+/[a-zA-Z0-9\-_]+
plumb start open https://github.com/$0

The rules work great -- if I select some such text in a file and press Enter, the page opens in my browser. 🎉

But I also wanted to not need to select the text and instead make use of the auto-expansion feature. In other words, if my cursor is positioned on some such foo/bar#123 reference without any selection, pressing Enter would auto-expand the dot to cover the whole text foo/bar#123 and then that would be matched against the plumbing rules and the link would be opened. So, exactly how it already works with URLs.

But to my surprise, pressing Enter only expanded the dot to one component of the reference, like this:

Screenshot 2024-10-14 at 14 06 33

Questions/discussion: I think I expected ad to be aware of my new plumbing rules and the matches regexes in them. And it would then update the dot auto-expansion rules to account for these rules as well to be able to match text to the rules.

After digging through the code a bit, I found expand_cur_dot which expands the current dot it it's a cursor, and as a part of that try_expand_known which expands the dot based on known patterns. As far as I can tell, the logic expand_cur_dot and try_expand_known has some hardcoded behavior for a few well-known patterns (like URIs and file paths), but it doesn't have the ability to take into account something that the user defined.

So, I'm wondering -- would it make sense to allow the expand_cur_dot / try_expand_known logic to be extended by user configuration? For example, could that happen automatically based on the regexes used in matches parts of plumbing rules? Or could that happen based on some other user-defined configuration in .ad/init.conf -- a list of regexes for auto-expanding dot? Or even both? 🤔 💭

sminez commented 3 weeks ago

Hi @izuzak :wave:

First things first: I'm 100% stealing that plumbing rule :smile:

I have this one at the moment which works if you have the gh CLI tool installed and are in the repo in question for things like gh#38 (this issue):

# open in the browser
data matches gh#(\d+)
plumb start gh issue view -w "$1"

# or alternatively pull the details and view in ad
data matches gh#(\d+)
data from gh issue view $1
attr add action=showdata filename=/GitHub/issue/$1
plumb to edit

As for your question itself, expand_cur_dot follows the behaviour present in acme (a phrase you'll find me saying a lot with this project :sweat_smile:) where it is deliberately limited to things that look like:

Importantly, it does not adjust its behaviour based on your plumbing rules: each matches statement within the ruleset is used to test a selection that has been Loaded, not determine the selection. Having the smart expand attempt to do more than this quickly causes issues with using it to search for things like struct and function names within a codebase, as you end up grabbing a larger region of text than you want a lot of the time.

I think rather than making expand_cur_dot more complicated it might be an alright compromise to have something that expanded until whitespace? That would let you grab the patterns you are showing in your GIF but it would need to be a different key binding / command. Doing so should be possible with a simple script that runs an Edit command I think?

That said, the expected user interaction in these cases is actually for you to explicitly select the text you want to load. Now that #35 has landed you can do this with the mouse by dragging the right mouse button which works pretty nicely and doesn't require too much in the ways of clever logic that you need to remember the quirks of inside of expand_cur_dot. Though I do concede that doing it with the keyboard can be a little clunky :shrug:

izuzak commented 3 weeks ago

I think rather than making expand_cur_dot more complicated it might be an alright compromise to have something that expanded until whitespace? That would let you grab the patterns you are showing in your GIF but it would need to be a different key binding / command. Doing so should be possible with a simple script that runs an Edit command I think?

I agree -- not changing existing behavior/expectations here (around expand_cur_dot) makes a lot of sense. And instead we should introduce something new/separate. And that New Thing would be triggered by a different key binding / command. 👍

Thinking about that New Thing, I think something that finds the whitespace-surrounded region would work for the use-cases I have on my mind right now. 👍

That said, I still wonder if this is a place where user configurability would be a useful option. Would such a point of configurability be aligned with the general goals/ideas for the editor or not really? 💭 Even if it would be aligned, I'm not saying it should be done now -- it could totally be a possible feature for the future. Everything here is low urgency for me since it's already possible if I do the selecting myself ✨

I also wonder if I might do something like this:

  1. press a custom keybinding
  2. invoke an external tool which finds the cursor and determines what the selection should be
  3. the tool selects the text in ad

That would basically do the same thing as my original idea, but this definition of what should be selected (via regexes or whatever) feels so tightly coupled to the editor itself that it seems unnecessary/overcomplicated. So, it seems to me (at least right now) that it's not a great case for "integrating" with external tools and would be a better case for "user customization" within the bounds of the editor.

rakoo commented 3 weeks ago

Hey there !

ad lurker here, I see that the discussion is considering "how to automatically select stuff" and it makes me think about the ki editor.

It's yet another vi-inspired editor with the following trick: instead of having a custom keybinding for each movement, the normal mode has an additional "selection mode" to choose from and that dictates movements: character, punctuation-delimited word, whitespace-delimited word, syntax node (thanks to tree-sitter), line, regex, diagnostics in LSP, ... Thanks to that there are only four movements (hjkl) and each mode will "set dot" to another object (in line mode it selects the next line, in word mode it selects the next word, etc...)

I think this approach is extremely interesting and can make manipulation of blocks far easier. The syntax-node mode is particularly interesting because it automagically selects the "right" thing most of the time. Now that of course doesn't mean ad should do the same, but I think it's important to keep in mind that going in the direction of "I want to select based on other criterias" can quickly become overwhelming, and the ki approach is nicely orthogonal and complementary

sminez commented 3 weeks ago

@izuzak I was trying to get an example recorded for you earlier and realised that I still needed to tackle #21 and in doing so realised there were a couple of bugs with how addresses were being handled in Edit scripts. Both of those are addressed (no pun intended) in #39 now.

we should introduce something new/separate

To clarify, we shouldn't :slightly_smiling_face: This is the point of acme and ad: the primitives from the editor should be enough to implement things like this for your specific use case rather than adding more and more bespoke pieces of functionality to the editor itself. (Acme purists will likely view the additions I've introduced in ad as being overkill already!)

In this specific case, the demo video in #39 and below should hopefully show you how the existing functionality can be used in order to implement something like "expand to whitespace delimited word" which you could then bind to a key in your init.conf. The addr file exposed through the virtual filesystem interface allows you to set the dot for a given buffer using a pretty rich syntax, which is also accepted by the Edit command (pressing . in NORMAL mode will drop you in to EDIT mode where you can write a structural regular expression to select and manipulate the buffer).

https://github.com/user-attachments/assets/0a8b416b-0885-497a-9a4b-ab1f64f51b5f

The demo uses the following addr expression:

-/\s/+#1,/\s/-#1

Which is a bit on the perl-y side so I'll try to explain :sweat_smile:

This ability to manipulate the current selection via the filesystem interface is the way that user extension of the editor is intended to work. The keybingings in init.conf are a nice quality of life improvement over the functionality exposed by acme but I'm not intending on adding much more (if at all) to the "built in" configuration beyond what is there today. I'm open to exploring whether or not additional built-in features are needed as the project evolves but an explicit goal is to keep things as minimal as possible.

Hopefully I'll have some time in the near future to add to the :help docs to cover these topics so that it is easier to learn about and discover the existing functionality.

sminez commented 3 weeks ago

Hi @rakoo :wave:

I've not played around with ki before but I have read up on it and the approach it takes to making selections is pretty fun. I think it definitely falls into the "another interesting take on how to interact with your editor" category though, rather than being something that I would add to ad. There are a lot of different attempts at rethinking how your editor should work out there and I think they're well worth exploring if you like the sound of them :slightly_smiling_face:

rakoo commented 2 weeks ago

For the sake of the discussion, acme is pretty good at identifying delimiters. A common trick for problems like this, typically for commands with arguments and so with spaces in them is to encapsulate them inside [ and ] and double-click inside any of the bracket. That sets dot to the entire string inside the brackets.

Since it's still a few more clicks and typing, another editor, Anvil has come up with ◊: when a string is encapsulated inside lozenges and you middle-click anywhere, the entire string inside lozenges is executed. It basically overrides matching rules to say "all of the text inside lozenges".

Again, not saying those should be built-in, but ideas for how to extend ad in ad-hoc (ha) programs :)

sminez commented 2 weeks ago

implementing double click at the first/last char inside of delimiters is something I'm hoping to take a look at soon :slightly_smiling_face:

In the meantime, alt-i $delimiter in normal mode will select between delimiters

sminez commented 2 weeks ago

Ok, so soon turned out to be this evening and it didn't take too long to sort out: 9cb6d80046e446603171f2a578aa1b7478e81c97

https://github.com/user-attachments/assets/04775b6e-f79b-423a-98c9-dbc2354ffcf5

rakoo commented 2 weeks ago

That's so lovely, thanks !

Regarding the gh tool, russ cox created the issue acme tool that might be useful to have a look at !

srenatus commented 2 weeks ago

There's a little mouse in the screencast now! 😍 Thank you!

sminez commented 2 weeks ago

@izuzak if it's alright with you I'd like to close this issue as resolved now. Now that the double click expand functionality is there, the acme idiom of wrapping the thing you want to select in delimiters (so (foo/bar#42)) if whitespace delimited double click isn't what you need is the way to handle this I think :slightly_smiling_face:

izuzak commented 2 weeks ago

@izuzak if it's alright with you I'd like to close this issue as resolved now.

Agreed! Many thanks for all the explanations, discussions, and improvements ✨