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

Store line change in script navigation history #63515

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jul 26, 2022

Closes #16554
Closes godotengine/godot-proposals#4989 (what a coincidence...)

Any movement with more than 10 lines apart is saved into history.
godot windows tools 64_qixu8sT8AL
The implementation is rather hacky and might be not 100% reliable, but script editors are a mess :/ At least it works.

@reduz
Copy link
Member

reduz commented Aug 11, 2022

I have the feeling I implemented this at some distant time in the past..

@Calinou
Copy link
Member

Calinou commented Aug 11, 2022

I have the feeling I implemented this at some distant time in the past..

You might be thinking about script tab temperature, which is a different feature that serves different needs.

@reduz
Copy link
Member

reduz commented Aug 11, 2022

@Calinou No I am pretty sure I made the history go back to the proper script/line.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 11, 2022

Yes, the history works this way, but the line change wasn't recorded in the history. Only script changes were and in 3.x Ctrl+clicks on members (which was probably a convenient bug lol).

@reduz
Copy link
Member

reduz commented Aug 11, 2022

Oh alright, in any case, up to @Paulb23 to review

Copy link
Member

@Paulb23 Paulb23 left a comment

Choose a reason for hiding this comment

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

Yep definitely a slightly hacky approach, using [set|get]_edit_state will also change breakpoints and bookmarks etc, so currently we could end up resting these as well, which is not ideal.

Should also probably add support for scrolling around the docs as well.

void ScriptTextEditor::_on_caret_moved() {
int current_line = code_editor->get_text_editor()->get_caret_line();
if (ABS(current_line - previous_line) > 10) {
emit_signal(SNAME("request_save_previous_state"), previous_line);
Copy link
Member

Choose a reason for hiding this comment

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

Could we emit the variant here, then we don't have to expose it to ScriptEditorPlugin

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there is a way to use a signal without exposing it, is there?

Copy link
Member

Choose a reason for hiding this comment

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

I mean't not exposing the [get|set]_previous_state methods by emitting the state directly here (only navigation state).

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 30, 2022

Should also probably add support for scrolling around the docs as well.

Docs don't have caret though.

EDIT:
Soooo

Yep definitely a slightly hacky approach, using [set|get]_edit_state will also change breakpoints and bookmarks etc, so currently we could end up resting these as well, which is not ideal.

I was testing this and yes, my deleted bookmarks get restored when I go back. I just hooked into already-existing script history, which unfortunately keeps everything for whatever reason. I think we should have something like "navigation state" that stores only caret position and scrolls. It would be a subset of the full state and used by the history. Then full state would be tied only to the script editors.

@Paulb23
Copy link
Member

Paulb23 commented Sep 19, 2022

For docs I was thinking along the lines of, clicking some in doc navigation such as a method in the same class. It would be nice to store that as a navigation action, would be v_scroll here rather the caret.

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 22, 2022

Updated. I changed to use the new navigation history and simplified storing a bit. Also I added some scroll saving to editor help, but it's unreliable.

There is a bug sometimes where a position can be saved twice in a history, but I don't know what triggers it.

@KoBeWi KoBeWi requested a review from a team as a code owner October 22, 2022 21:56
@akien-mga akien-mga requested a review from Paulb23 January 6, 2023 07:12
@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023
@KoBeWi
Copy link
Member Author

KoBeWi commented Mar 22, 2023

I did some more tweaks and I think this works reliably now.

@KoBeWi KoBeWi force-pushed the script_jumper branch 2 times, most recently from a4bda9d to e037705 Compare March 26, 2023 20:34
@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@AThousandShips AThousandShips modified the milestones: 4.2, 4.3 Oct 26, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Tested, seems to work fine.

@akien-mga akien-mga dismissed Paulb23’s stale review April 26, 2024 13:12

Changes done.

@akien-mga akien-mga merged commit 7cb52a6 into godotengine:master Apr 26, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

Push jumps to script editor history Feature Suggestion for Code Editor: "Jump to last edit location"
7 participants