Add trimSelection parameter to text editor

[pmario]

As discussed at: https://talk.tiddlywiki.org/t/ideas-to-improve-wrap-selection-and-wrap-lines-operations-tm-edit-text-operation/8935

This PR adds a new parameter to the "wrap-selection" editor button functions.

title: $:/core/ui/EditorToolbar/bold

<$action-sendmessage
    $message="tm-edit-text-operation"
    $param="wrap-selection"
    prefix="''"
    suffix="''"
    trimSelection="yes"  <!-- no (default), yes, start, end - are possible -->
/>

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
tiddlywiki5	✅ Ready (Inspect)	Visit Preview	Mar 30, 2024 7:31am

[pmario]

@Jermolene please test with macOS / Safari - I do not have that possibility. -- This PR is a draft atm, so users can play with it

[pmario]

I also did a short test with CodeMirro5 plugin -> Seems to work as expected.

[mateuszwilczek]

@pmario it works as expected with the core toolbar buttons in default editor. However, I cannot make it work with my custom buttons that add markup other than the core buttons. I tried the following in the preview:

• adding trimSelection="both" to my custom button
• cloning and modifying a core button, e.g. bold, so that it adds custom markup, e.g. @@ prefix and suffix
• I tried saving and reloading in each case

The "ignoring" of spaces does not work, so if word is selected, then it is changed to @@word @@. However, removing of markup even if the inline markup is not selected does work, so if only word inside @@word @@ is selected, it will be changed to word as expected.

Is your current solution by any chance hard coded to support only the markup symbols present in core toolbar buttons, but not any arbitrary prefix/suffix? The ignoring of spaces in selection does work if I change the prefix/suffix to something wild that uses "default" symbols only, e.g. ///'_,~, but does not work with @@.

[pmario]

@mateuszwilczek

• go to https://tiddlywiki5-git-fork-pmario-trim-selection-tex-1760fa-jermolene.vercel.app/#How%20to%20create%20dynamic%20editor%20toolbar%20buttons
• click the "Create temporary bold button"
• change prefix and suffix to @@
• Save the tiddler

It should work as expected. I did just test it and it works for me.

[mateuszwilczek]

@pmario I got where the problem is. There's a typo in the parameter name, c is missing in trimSeletion="both".
In my attempts I was copying the parameter from the first post here trimSelection="both", which has no typo.

[mateuszwilczek]

Since the only problem I found has been identified, I think its a good improvement. I like that it works on all whitespace, not just the single trailing space selected by double click on Windows. So it also works in cases with a lot of different whitespace characters on both ends of the selection.
E.g. this becomes this

As to the name of the parameter and possible values, perhaps something like skipWhitespace would be more clear? As you pointed out, it does not "trim" or "remove" the spaces, just "skips" or "ignores" or "omits" the surrounding spaces when applying markup.
The possible values would be default no (for backwards compatibility) and yes, instead of both. I don't think that the specific start/end only options are necessary.

The only thing I could wish for, would be to identify markup to remove if it is partially selected. Core removes markup if all of it is selected, e.g. //example//. Your PR makes it so that markup is also removed if only its "content" is selected, e.g. //example//.
It would be nice if partial selection, e.g. //example// or //example// would also make the markup be removed. I realize it might be difficult to implement, and as said, this would be a luxury to have. The features that you have already implemented are much more important.

[pmario]

The possible values would be default no (for backwards compatibility) and yes, instead of both. I don't think that the specific start/end only options are necessary.

We could change both to yes.

But I think start and end should stay. I'm pretty sure users will have usecases where they will make sense. We just don't know them yet.

hmmm skipWhitespace may be an option.

@Jermolene @saqimtiaz ... any ideas?

[pmario]

I did change the prameter "both" to "yes" - Switch this PR to "ready for review" since there was no other feedback

[pmario]

@Jermolene -- please review again. IMO this PR is a nice quality of live improvement, which also fixes a constant annoyance if macro parameters are double-clicked and renamed.

[Jermolene]

Thanks @pmario just some coding style points

[Jermolene]

I'm not a huge fan of this abbreviation pattern. I understand that it saves a bit of space, but it is hard to apply consistently. For example, here one might wonder why "operation" is abbreviated but "prefix" is not.

On balance, I would prefer to find ways to refactor the code to avoid the repetition, rather than use this pattern.

[pmario]

prefix is an abbreviation. It is the shorthand for event.paramObject.prefix

[pmario]

The "o" abbreviation is used 99 times and was introduced to make the code readable and understandable. If I add it back again, it will make the code completely unreadable. Reducing the file size by 800bytes is a side effect.

IMO there is no redundant, repettitive code on the right side of any =. Every line, that defines o.cutStart, o.cutEnd, ... and so on, has a different pattern on the right side. -- So I do not really see a possility to optimize that any further.

[pmario]

@Jermolene -- Please consider this one again. It is annoying on Widows if a word is double clicked and then CTRL-B for bold is uses.

Since Windows does select the word and the following whitespace, the current behaviour will include the withespace within the formatting. Which then needs to be fixed manually.

---

About your concerns of abbreviating operation. with o. -- I did "undo" it. But that makes the code completely unreadable and it is not possible to understand it anymore. As I wrote operations is needed 99 times and if written in full length it completely "hides" the logic, which is defined by selection

As I wrote. I think, there is no duplicated code. The right side of = is different depending on actual selection and user configuration. The left side are the parameter the operation object needs to work. So they have to be defined.

[pmario]

I did test it with the default and the codemirror editor. Both work in the same way.

[Jermolene]

Thanks @pmario