Enhance Matrix Shortcut with Multi-selection #394
Conversation
Testudinidae
force-pushed
the
feature/matrix-shortcut
branch
from
e468f2c to
d40a3b9
Compare
|
Probably want to rebase and squash merge commits, as well as refactor commits. |
Testudinidae
force-pushed
the
feature/matrix-shortcut
branch
from
d40a3b9 to
54a0e20
Compare
|
I might reorganize the git commit history in the next couple of days. Feel free to point out any unexpected behavior. |
Testudinidae
force-pushed
the
feature/matrix-shortcut
branch
from
54a0e20 to
8484466
Compare
…get-environment-bound
…nt' into feature/matrix-shortcuts-trim-empty-line-after-environment
Testudinidae
force-pushed
the
feature/matrix-shortcut
branch
from
8484466 to
0a2cd87
Compare
…-shortcuts-trim-extra-align
…e/matrix-shortcut
Testudinidae
force-pushed
the
feature/matrix-shortcut
branch
from
0a2cd87 to
46f495b
Compare
superle3
left a comment
superle3
left a comment
There was a problem hiding this comment.
will review further some time this week.
src/features/matrix_shortcuts.ts
Outdated
| if (newPos > envBound.end) { | ||
| if (trimEmptyLineAfterEnv && line.text.trim() === "") { | ||
| replaceRange(view, line.from, nextLine.from, ""); | ||
|
|
||
| d = view.state.doc; | ||
| lineNo--; | ||
| line = d.line(lineNo); | ||
| nextLine = d.line(lineNo + 1); | ||
| newPos = nextLine.to; | ||
| } | ||
|
|
||
| if (addLineBreakAfterEnv && !envText.trimEnd().endsWith("\\\\")) { | ||
| setCursor(view, line.to); | ||
|
|
||
| applySeparator(END_LINE_BREAK, view); | ||
|
|
||
| d = view.state.doc; | ||
| nextLine = d.line(lineNo + 1); | ||
| newPos = nextLine.to; | ||
| } | ||
| } | ||
|
|
||
| setCursor(view, newPos); | ||
| } |
There was a problem hiding this comment.
I know this may make things complex, but consider applying only one dispatch instead of 2-3.
The functions that dispatch things are:
applySeparator, setCursor, andreplaceRange.
Either you can collect transactions and compose changes to those so you can keep the logic together or consider decoupling the logic.
Multiple dispatches have an impact on performance, for sure if the document is large/ lots of text is in the viewport.
| if (trimWhitespace) { | ||
| // If at the beginning of the line | ||
| if (textBeforeFrom === "") { | ||
| separator = separator.match(/^[ \t]*([\s\S]*)$/)[1]; |
There was a problem hiding this comment.
why custom function over trimStart? why match over replace and is this preparing for user defined separators or smth? because the separators above don't have a leading newline.
| const isMultiLineBreak = (separator: string): boolean => { | ||
| return separator.contains("\\\\") && separator.contains("\n"); | ||
| } | ||
|
|
There was a problem hiding this comment.
is there a specific reason why you're not checking for LINE_BREAK or just
const regex = /\\\\\s*\n/or smth? What custom separator did you have in mind? because there is no reason to have this generalization in the current version.
|
I think thats all? Not experienced with reviewing so if some comments are too outrageous or unclear, just let me know. (for example I am not sure how much I should govern the style, hopefully everything I said was reasonable?) |
Changed `d.lineAt` from `range.from` to `range.to` to correctly compute `toLine`.
- Use `ChangeSet` and `changeByRange` in `applySeparator` to apply separator changes, update selections and remap cursors. - Flatten matrix shortcut control flow for clearer, more linear logic. - Update `generateSeparatorChange` to take `doc` and `settings` instead of the full `view`. - Rename some variables for better readability.
Testudinidae
force-pushed
the
feature/matrix-shortcut
branch
from
b8275c0 to
adeafbe
Compare
|
Hi @Testudinidae , (Not trying to rush you, it's oss and volunteer work so things take time/ take your time) I am not in a hurry/dont have a lot of free time this week, but do you mind me merging other prs and make a release since this pr is in draft or were you planning on completing it soon? Also since you resolved every comment in the other pr (except the last one but that doesn't need to be answered before merging), does that mean I can merge it? |
3 participants
This is a significant update with multiple enhancements and refactors to improve matrix shortcut handling and internal logic. Below is a summary of the changes made:
Users can now use multi-cursor with selection within matrix shortcuts, enabling more efficient edits.
isWithinEnvironmentThe
isWithinEnvironmentfunction has been split into two separate functions:getEnvironmentBoundisWithinEnvironmentThis refactor improves clarity and enhances the function's usability.
\hlineThe logic now ignores adding a line break (
\\) after encountering\hline, ensuring no unnecessary line breaks are inserted in these situations.Optional Features:
true)When enabled, extra spaces around the cursor are trimmed automatically. This helps address common cases where users accidentally add extra spaces, either by pressing the spacebar unintentionally or due to snippets automatically adding them. For example, pressing Tab after
"1(a lot of space)"will result in"1 & "rather than"1(a lot of space) & "to prevent accidental spaces from accumulating.true)This trims unnecessary alignment symbols that can occur if users accidentally press Tab at the end of a line. For instance, pressing Enter after
"1 & "could result in an extra"&"being inserted, but with this feature enabled, it will correctly turn into"1 \\"instead of"1 & \\"to avoid such mistakes.\\After\hline(defaultfalse)If enabled, a line break
\\will be inserted after\hline.true)When exiting an environment using Shift+Enter, an empty line after the environment is removed to prevent unnecessary blank spaces. For instance:
... 1 & 2 & 3 \\ <- (cursor) \end{pmatrix}would transform to:
... 1 & 2 & 3 \\ \end{pmatrix} <- (cursor)false)If enabled, Shift+Enter will add a line break after exiting an environment. For example:
... 1 & 2 & 3 <- (cursor) \end{pmatrix}will transform to:
... 1 & 2 & 3 \\ \end{pmatrix} <- (cursor)These updates aim to enhance the user experience and provide better control over formatting and matrix shortcuts.