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

Add option to sort script list by last opened #83608

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

L4Vo5
Copy link
Contributor

@L4Vo5 L4Vo5 commented Oct 19, 2023

Closes godotengine/godot-proposals#8125
Added "Last Opened" option to text_editor/script_list/sort_scripts_by. I put it after "None", which looks strange, so that it's backwards compatible.
Added an EditorSetting called last_opened_script_threshold. A selected script won't be moved to the top if it's already within the first N scripts on the list.
Also added documentation for both.

I had to change script_close_queue to reference the Control nodes directly instead of the index, because scripts are selected during the closing process, which shuffled them around and invalidated the previous indices in some cases.

This also changes help classes to not add themselves to the history when loading a project. This was necessary to make them not end up at the top with the new option.
To be clear, the behaviour used to be that when you loaded a project, the script history would be entirely populated by all the opened doc classes, in order. So I reckon it wasn't intended anyway :p

I also avoid sorting the script list in _update_script_names if not on an effective sorting type. So, I set lock_history to false after the loop because otherwise it'd stay true after being set in _update_history_pos. I'm not sure how _update_history_pos works or why it sets it to true, so I figured this was the safest place to set it to false.

@L4Vo5 L4Vo5 requested a review from a team as a code owner October 19, 2023 09:47
@AThousandShips AThousandShips added this to the 4.x milestone Oct 19, 2023
@L4Vo5 L4Vo5 marked this pull request as draft November 29, 2023 20:11
@L4Vo5 L4Vo5 force-pushed the sort-scripts-by-recent branch from 1289e5a to 3f66c74 Compare August 4, 2024 04:03
@L4Vo5 L4Vo5 marked this pull request as ready for review August 4, 2024 04:07
@L4Vo5
Copy link
Contributor Author

L4Vo5 commented Aug 4, 2024

Alright, I got around to fixing the couple issues this had, so I took it out of draft status and updated the description.

@L4Vo5 L4Vo5 force-pushed the sort-scripts-by-recent branch from 3f66c74 to 923ed7f Compare August 4, 2024 06:22
@Calinou
Copy link
Member

Calinou commented Aug 4, 2024

Wouldn't the term "Last Opened" be more relevant here than "Recency", especially since we use a similar name in the project manager order setting?

@L4Vo5
Copy link
Contributor Author

L4Vo5 commented Aug 4, 2024

Wouldn't the term "Last Opened" be more relevant here than "Recency", especially since we use a similar name in the project manager order setting?

I wouldn't say this sorts by "last opened", because clicking an already-opened script also moves it to the top.
I guess clicking a script is "opening the tab", but IMO it'd be confusing.
Maybe "Last Accessed" or "Last Viewed" would be better?

..what I'd really love a better name for is script_recency_ignore_threshold :,)

@L4Vo5 L4Vo5 force-pushed the sort-scripts-by-recent branch from 923ed7f to 9102def Compare August 8, 2024 07:51
@L4Vo5
Copy link
Contributor Author

L4Vo5 commented Aug 8, 2024

I decided to rename "Recency" to "Last Viewed" and "script_recency_ignore_threshold" to "last_viewed_script_threshold".

@ZenRepublic
Copy link

thank you, this is better than godot 5

@Mickeon
Copy link
Contributor

Mickeon commented Sep 22, 2024

There's some additional interest for this. The PR could use a rebase. I'd also rename the option to "Last Opened".

@L4Vo5 L4Vo5 force-pushed the sort-scripts-by-recent branch from 9102def to 63ea06e Compare September 25, 2024 18:33
@L4Vo5 L4Vo5 requested a review from a team as a code owner September 25, 2024 18:33
@L4Vo5 L4Vo5 changed the title Add option to sort script list by recency Add option to sort script list by last opened Sep 25, 2024
@L4Vo5
Copy link
Contributor Author

L4Vo5 commented Sep 25, 2024

Done, also added documentation thanks to #93427

@L4Vo5 L4Vo5 force-pushed the sort-scripts-by-recent branch from 63ea06e to 4bcb6ad Compare September 28, 2024 00:17
@L4Vo5 L4Vo5 requested a review from a team as a code owner September 28, 2024 00:17
Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

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

Documentation is great. I cannot speak on the actual implementation. This could use another rebase for hopefully testing.

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.

Add an option to sort script list by most recent
5 participants