Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Goto line a Popup and column input #91388

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

kitbdev
Copy link
Contributor

@kitbdev kitbdev commented May 1, 2024

Changes Goto Line Dialog to be a popup, since it should be transient. Clicking outside will close it.
Allows inputting column after the line, after a :. This is actually the character position, so tabs count as 1 and not 4. It also starts at 1, like the line number. If the column number is too high or too low, it will be clamped to the line length.

Live navigation, the editor will move to the line immediately without needing to confirm.
Pressing Enter closes the popup. Even if there is no number.
Pressing Esc closes and returns to the original position (using navigation state).
The default popup position is at the top center of the TextEditor it is on.

Fixes Goto line not deselecting.
Goto Line now centers to the line and sets the caret column to 0.

image

@KoBeWi
Copy link
Member

KoBeWi commented Oct 24, 2024

Using EditorSpinSlider won't allow inputting columns (e.g. 100:20) if we ever want to support that.

@kitbdev
Copy link
Contributor Author

kitbdev commented Oct 24, 2024

Yeah, I think column support is probably more important than expression support here.
The EditorSpinSlider isn't the best fit anyway, since it starts focused and the buttons on it are only available after unfocusing it with tab.
It would also be nice to have live changes.
I'll make some changes.

@kitbdev kitbdev changed the title Make Goto line a Popup and allow expressions Make Goto line a Popup and column input Oct 24, 2024
@kitbdev
Copy link
Contributor Author

kitbdev commented Oct 24, 2024

Updated

  • allows column number input.
  • goes to line immediately (can cancel to return).
  • no longer supports expressions.

@KoBeWi
Copy link
Member

KoBeWi commented Oct 30, 2024

Closing the popup by clicking will select text:

godot.windows.editor.dev.x86_64_s32gP0CKPS.mp4

@kitbdev
Copy link
Contributor Author

kitbdev commented Oct 30, 2024

That looks like a preexisting issue:

It would have to fixed in the TextEdit itself, and I think it can be fixed separately.

editor/code_editor.cpp Outdated Show resolved Hide resolved
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally (rebased on top of master 7187c25), it works as expected.

I noticed a few UX issues though:

  • The undo/redo navigation will take all steps of the action into account, including when you write the first digits of the line number. For instance, if you want to go to line 123, there will be 3 actions stored:

    • Go to line 1
    • Go to line 12
    • Go to line 123
  • To go back to the location you were at previously, you need to undo the navigation 3 times, even if you press Escape and don't confirm the dialog.

    • I like the idea of this "live preview", but it should only store the navigation state after you confirm the dialog by pressing Enter.
  • Pressing Escape after entering some numbers should also restore the old navigation history. Right now, when you enter numbers then press Escape, the numbers you entered up to that point will be part of the navigation history, even though you were (correctly) sent back to the location you were at before entering anything.

@kitbdev
Copy link
Contributor Author

kitbdev commented Oct 31, 2024

Updated so previewing line changes does not affect the navigation history.

editor/code_editor.cpp Outdated Show resolved Hide resolved
@Repiteo Repiteo modified the milestones: 4.x, 4.4 Dec 23, 2024
@Repiteo Repiteo merged commit 98c5267 into godotengine:master Dec 23, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 23, 2024

Thanks!

@kitbdev kitbdev deleted the fix-goto-line branch December 24, 2024 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants