feat(editor): indent selected text on TAB key event #482

itsaky · 2023-09-07T06:44:01Z

This adds the ability to indent selected text when the TAB key is pressed. The feature can be enabled/disabled using DirectAccessProps.indentSelectionWithTab.

Related issue : AndroidIDEOfficial/AndroidIDE#1229

Current behavior

Record_2023-09-07-12-09-19.mp4

New behavior

Record_2023-09-07-12-07-56.mp4

itsaky · 2023-09-07T15:27:20Z

Converting this to a draft. Will make it "ready for review" once this feature is implemented.

editor/src/main/java/io/github/rosemoe/sora/widget/EditorKeyEventHandler.java

Rosemoe · 2023-09-07T16:22:07Z

editor/src/main/java/io/github/rosemoe/sora/widget/SymbolInputView.java

+                            editor.getSnippetController().shiftToNextTabStop();
+                        } else if (editor.getProps().indentSelectionWithTab) {
+                            editor.indentSelection();
+                        }


Missing fallback case for inserting the TAB.

Sure, will fix this.

Rosemoe · 2023-09-07T16:23:52Z

editor/src/main/java/io/github/rosemoe/sora/widget/SymbolInputView.java

+                    if ("\t".equals(insertText[finalI])) {
+                        if (editor.getSnippetController().isInSnippet()) {
+                            editor.getSnippetController().shiftToNextTabStop();
+                        } else if (editor.getProps().indentSelectionWithTab) {


Additional conditions as above should be added here.

Didn't get this. What do you mean by "additional conditions as above" (the single line thing?)?

Sorry for the ambiguous statement. I mean additional check for if text is selected.

Oh, okay. I'll add it.

…l input view

…elected] lines

gituser1000000 · 2023-09-08T15:39:32Z

What do you think of these two choices?

Shift-TAB moves line back until the number of leading spaces is less than tabWidth. [how it is now]
Shift-TAB moves line back until the number of leading spaces is zero.

One of the second choice's benefits is that if the line is not currently at an aligned stop (not at 1xtabWidth, 2xtabWidth, 3xtabWidth, ...), we could (if we need) give it a final Shift-TAB key press to align it with the left edge. We could then TAB it to where we want it to be.

Also, another benefit is being able to move the selection to the start of the line. With option 1, if it's not already aligned with a multiple of tabWidth, we could not do this.

Rosemoe · 2023-09-12T09:16:35Z

The configuration from language (useTab character) and editor tab width should also be considered when indenting/unindenting the texts.
Currently TAB and Space characters are the same in your implementation.🤔

Rosemoe · 2023-09-12T09:24:51Z

What do you think of these two choices?

Shift-TAB moves line back until the number of leading spaces is less than tabWidth. [how it is now]

Shift-TAB moves line back until the number of leading spaces is zero.

One of the second choice's benefits is that if the line is not currently at an aligned stop (not at 1xtabWidth, 2xtabWidth, 3xtabWidth, ...), we could (if we need) give it a final Shift-TAB key press to align it with the left edge. We could then TAB it to where we want it to be.

Also, another benefit is being able to move the selection to the start of the line. With option 1, if it's not already aligned with a multiple of tabWidth, we could not do this.

In vscode, SHIFT + TAB actually deletes leading space to align the indentation to nearest KxTabwidth.
For example, if we unindent on a line with 5 leading spaces, vscode deletes one space to make it 4 leading spaces.
I think this logic is better.

gituser1000000 · 2023-09-12T14:56:01Z

The point of my previous post's concern was making sure it could (and should) finally arrive at the start of the line.

If the point was about whether we should align the line to a tab stop first (the same as VSCode as you mentioned) or right before reaching the start of the line, then yes, I agree that doing it first is better.

Another thing is when we say to the "nearest" KxTabWidth, it should mean "nearest on the left," correct? Otherwise, we might never reach the start of the line. When doing a regular TAB, then it's vice-versa, nearest on the right. Maybe it was already obvious, but I would just like to make it clear. :)

Rosemoe · 2023-09-13T12:28:05Z

Another thing is when we say to the "nearest" KxTabWidth, it should mean "nearest on the left," correct? Otherwise, we might never reach the start of the line. When doing a regular TAB, then it's vice-versa, nearest on the right. Maybe it was already obvious, but I would just like to make it clear. :)

Yes.

itsaky · 2023-09-13T16:54:36Z

Currently TAB and Space characters are the same in your implementation.🤔

Only in the un-indentation, right?

I'll work on suggestions posted here. But just to be sure, are the following statements correct? If not, please specify the expected behavior.

TAB should indent the selected lines to the next-nearest tab stop. If the line is already at a tab stop, the indentation should be incremented.
- This is assumed from @Rosemoe's statement here.
Shift + TAB should move the indentation of the lines (all selected lines or the line at cursor) to the previous-nearest tab stop. If the line is already at a tab stop, the indentation should be decremented.

Should the current behavior be preserved (i.e. should I add a property to toggle between the current behavior and the new behavior)?

gituser1000001 · 2023-09-14T06:11:21Z

Let me try my best to summarize what we have so far.

Example:

tabWidth is 4
Selected lines
[9sp]if (true) {
[13sp]intVar = 0;
[9sp]}

With new behavior:
PressingTab

1 time

[12sp]if (true) {
[16sp]intVar = 0;
[12sp]}

2 times

[16sp]if (true) {
[20sp]intVar = 0;
[16sp]}

or,
Pressing Shift+Tab
1 time

[8sp]if (true) {
[12sp]intVar = 0;
[8sp]}

2 times

[4sp]if (true) {
[8sp]intVar = 0;
[4sp]}

3 times

[0sp]if (true) {
[4sp]intVar = 0;
[0sp]}

4 times

Stays the same as at 3 times

With old behavior (make sure the selection is able to arrive at the left edge):
PressingTab

1 time

[13sp]if (true) {
[17sp]intVar = 0;
[13sp]}

2 times

[17sp]if (true) {
[21sp]intVar = 0;
[17sp]}

or,
Pressing Shift+Tab
1 time

[5sp]if (true) {
[9sp]intVar = 0;
[5sp]}

2 times

[1sp]if (true) {
[5sp]intVar = 0;
[1sp]}

3 times

[0sp]if (true) {
[4sp]intVar = 0;
[0sp]}

4 times

Stays the same as at 3 times

The new behavior should be the default as more users might prefer it.

@itsaky
In regard to being able to move the selection by one space at a time (as requested before in the AndroidIDE repository), maybe it doesn't have to be too complicated to add. This would enable the users to micro manage the position of the selected text instead of always having to move it by one whole tabWidth.

What I have in mind is this.

User presses a UI button to quickly toggle between using space or tab for the Tab/Shift+Tab keybindings.
When Tab/Shift+Tab is pressed, if using space, we would simply move the selection by a space. If using tab, we would just move the selection by a tab as normal.

Rosemoe · 2023-09-15T10:41:29Z

tabWidth is 4

Selected lines
[9sp]if (true) {
[13sp]intVar = 0;
[9sp]}

With new behavior: Pressing Shift+Tab 1 time

[8sp]if (true) {
[12sp]intVar = 0;
[8sp]}

2 times

[4sp]if (true) {
[8sp]intVar = 0;
[4sp]}

3 times

[0sp]if (true) {
[4sp]intVar = 0;
[0sp]}

4 times

Stays the same as at 3 times

4 times case should be:

[0sp]if (true) {
[0sp]intVar = 0;
[0sp]}

Rosemoe · 2023-09-15T10:48:22Z

Only in the un-indentation, right?

Yes, this happens in un-indentation.

I'll work on suggestions posted here. But just to be sure, are the following statements correct? If not, please specify the expected behavior.

All right.

Should the current behavior be preserved (i.e. should I add a property to toggle between the current behavior and the new behavior)?

Don't have to preserve I think.

gituser1000001 · 2023-09-15T13:14:41Z

4 times case should be:

[0sp]if (true) {
[0sp]intVar = 0;
[0sp]}

Shouldn't we keep the selected text's original formation and stop moving as soon as at least 1 line reaches the start of the line?

I guess we could continue until everything has aligned with the left edge. However, the user needs to be careful not to accidently press even one too many Shift+Tab and ruin the original formation. Simply pressing Tab after doing a one-too-many won't reverse the action. The user would need to make a new selection to fix that.

If we could save the formation on a Shift+Tab, then for only the very next Tab, we would just restore that information. This could complicate things a bit.

Rosemoe · 2023-09-15T13:35:11Z

4 times case should be:

[0sp]if (true) {
[0sp]intVar = 0;
[0sp]}

Shouldn't we keep the selected text's original formation and stop moving as soon as at least 1 line reaches the start of the line?

At least, vscode will continue to reduce the indentation.

itsaky · 2023-09-15T13:53:48Z

4 times case should be:

[0sp]if (true) {
[0sp]intVar = 0;
[0sp]}

Okay, thanks for the confirmation.

Should the current behavior be preserved (i.e. should I add a property to toggle between the current behavior and the new behavior)?

Don't have to preserve I think.

Okay.

Shouldn't we keep the selected text's original formation and stop moving as soon as at least 1 line reaches the start of the line?

We can't know which line's indentation must be preserved unless we ask the language implementations to provide this information (which will be slow as the languages might need to analyze the source code). However, the [un]indentSelection methods are overridable so subclasses can specify their own behavior.

itsaky · 2023-09-19T14:21:07Z

The behavior has been updated according to the suggestions provided here. Here is a video demonstrating the same :

VID_20230919194123.mp4

Also, the TAB key event now behaves the same (doesn't matter text is selected or not).

feat(editor): indent selected text on TAB key event

fe05ee1

itsaky mentioned this pull request Sep 7, 2023

[Feature]: Space, backspace, tab, and back-tab the whole selection AndroidIDEOfficial/AndroidIDE#1229

Closed

2 tasks

itsaky marked this pull request as draft September 7, 2023 15:27

Rosemoe reviewed Sep 7, 2023

View reviewed changes

editor/src/main/java/io/github/rosemoe/sora/widget/EditorKeyEventHandler.java Outdated Show resolved Hide resolved

Rosemoe reviewed Sep 7, 2023

View reviewed changes

itsaky added 5 commits September 8, 2023 13:31

fix: default case for '\t' is not handled in symbol input view

150ce66

fix: add check for text selection before indenting selection in symbo…

dae64ac

…l input view

feat(editor): add Shift+TAB keybinding for removing indentation on [s…

d6e42e0

…elected] lines

docs: add documentation for new keybindings

ec390fd

fix: indentation is not removed if blank chars at start is < tabWidth

f5cb9fb

itsaky marked this pull request as ready for review September 8, 2023 09:04

itsaky requested a review from Rosemoe September 8, 2023 09:05

itsaky marked this pull request as draft September 15, 2023 15:45

itsaky added 3 commits September 19, 2023 18:45

fix: update behavior or Selection+TAB & Shift+TAB

e75d8d5

feat: use selection+TAB behavior for simple TAB key events

acc6748

fix: incorrect implementation of unindentSelection()

e99d581

fix: update JavaDoc

6677aea

itsaky marked this pull request as ready for review September 19, 2023 14:21

Rosemoe merged commit bcf3e90 into Rosemoe:main Sep 24, 2023
2 checks passed

itsaky deleted the indent-selection-with-tab branch September 25, 2023 13:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(editor): indent selected text on TAB key event #482

feat(editor): indent selected text on TAB key event #482

itsaky commented Sep 7, 2023 •

edited

Loading

itsaky commented Sep 7, 2023

Rosemoe Sep 7, 2023

itsaky Sep 7, 2023

Rosemoe Sep 7, 2023

itsaky Sep 7, 2023

Rosemoe Sep 7, 2023

itsaky Sep 7, 2023

gituser1000000 commented Sep 8, 2023

Rosemoe commented Sep 12, 2023

Rosemoe commented Sep 12, 2023

gituser1000000 commented Sep 12, 2023

Rosemoe commented Sep 13, 2023

itsaky commented Sep 13, 2023

gituser1000001 commented Sep 14, 2023 •

edited

Loading

Rosemoe commented Sep 15, 2023

Rosemoe commented Sep 15, 2023

gituser1000001 commented Sep 15, 2023 •

edited

Loading

Rosemoe commented Sep 15, 2023

itsaky commented Sep 15, 2023

itsaky commented Sep 19, 2023

feat(editor): indent selected text on TAB key event #482

feat(editor): indent selected text on TAB key event #482

Conversation

itsaky commented Sep 7, 2023 • edited Loading

Current behavior

New behavior

itsaky commented Sep 7, 2023

Rosemoe Sep 7, 2023

Choose a reason for hiding this comment

itsaky Sep 7, 2023

Choose a reason for hiding this comment

Rosemoe Sep 7, 2023

Choose a reason for hiding this comment

itsaky Sep 7, 2023

Choose a reason for hiding this comment

Rosemoe Sep 7, 2023

Choose a reason for hiding this comment

itsaky Sep 7, 2023

Choose a reason for hiding this comment

gituser1000000 commented Sep 8, 2023

Rosemoe commented Sep 12, 2023

Rosemoe commented Sep 12, 2023

gituser1000000 commented Sep 12, 2023

Rosemoe commented Sep 13, 2023

itsaky commented Sep 13, 2023

gituser1000001 commented Sep 14, 2023 • edited Loading

Rosemoe commented Sep 15, 2023

Rosemoe commented Sep 15, 2023

gituser1000001 commented Sep 15, 2023 • edited Loading

Rosemoe commented Sep 15, 2023

itsaky commented Sep 15, 2023

itsaky commented Sep 19, 2023

itsaky commented Sep 7, 2023 •

edited

Loading

gituser1000001 commented Sep 14, 2023 •

edited

Loading

gituser1000001 commented Sep 15, 2023 •

edited

Loading