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 exclude addons in Quick Open dialog #47563

Closed

Conversation

hilfazer
Copy link
Contributor

@hilfazer hilfazer commented Apr 2, 2021

One of the features requested in godotengine/godot-proposals#25.
A video with the feature working in 3.3:

Godot_exclude_addons.mp4

@Calinou Calinou added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Apr 2, 2021
@Calinou Calinou added this to the 4.0 milestone Apr 2, 2021
}
}

void EditorQuickOpen::_theme_changed() {
search_box->set_right_icon(search_options->get_theme_icon("Search", "EditorIcons"));
}

void EditorQuickOpen::_save_exclude_addons() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a project setting but also a checkbox in the dialog? I am against introducing checkboxes to Quick Open dialogs, but a project setting could be fine. That would allow this filter to be done just once when opening the dialog in _build_search_cache.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Imagine the user doesn't want to see addon files in his/her current project. But there came a time when s/he needs to search for a file from addons (for whatever reason). S/he opens project settings, find the setting, clicks it, opens the Quick Open dialog, finds the file s/he needs, opens the project settings again, finds the setting and clicks it again.
Unless there's a checkbox for than in Quick Open dialog that simplifies that process a lot. Which is the reason i've added it.

Copy link
Member

@Calinou Calinou Apr 3, 2021

Choose a reason for hiding this comment

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

I agree, it makes sense to expose this in a checkbox in the Quick Open dialogs. Its value should persist across restarts.

However, I'd store this in the editor settings rather than the project settings. You most likely don't want to commit that setting to version control whenever it changes 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it a project setting since the user might have some project when s/he doesn't want to see addons' files and some projects (like addons s/he's developing) where s/he does. But if editor settings is a better place for that, sure, i can change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hilfazer I don't imagine Quick Open is the first thing they will use to find it, I imagine they will just use the filesystem dock. But even this unlikely scenario you describe doesn't sound unreasonable. It's certainly preferable over introducing edge case checkboxes in every place, especially Quick Open dialogs. Unnecessary elements like this are a distraction that go against its purpose.

@hilfazer hilfazer force-pushed the quick_open_exclude_addons_40 branch 2 times, most recently from 81f3ded to 98d6c89 Compare July 4, 2021 12:56
@hilfazer
Copy link
Contributor Author

hilfazer commented Jul 4, 2021

I've replaced project setting with an editor setting. I'm not sure if "filesystem/file_dialog/quick_open_exclude_addons" is a right way to name it so i'm asking for some guidance here.

Hinlopen made a good point about using the filesystem dock so i've removed the checkbox.

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 against master in https://github.com/Calinou/godot/tree/quick_open_exclude_addons_40), it works as expected.

Setting disabled (default)

Screenshot_20230802_222341

Setting enabled

Screenshot_20230802_222353

Code looks good to me.

@YuriSizov
Copy link
Contributor

I feel like this option is a bit too specific. Are we going to add a new boolean for each case where we may want to exclude the addons folder? And the other way around, what if there is a use case for ignoring more folders for "Quick Open"?

@akien-mga akien-mga changed the title An option to exclude addons in Quick Open dialog Add option to exclude addons in Quick Open dialog Aug 3, 2023
@hilfazer
Copy link
Contributor Author

hilfazer commented Aug 5, 2023

I feel like this option is a bit too specific. Are we going to add a new boolean for each case where we may want to exclude the addons folder? And the other way around, what if there is a use case for ignoring more folders for "Quick Open"?

In case there should be a list of folders to ignore should it still be an editor setting or rather a project setting? While addons/ is universal across all projects the others folders a user might want to ignore will/can differ from project to project.

@hilfazer hilfazer force-pushed the quick_open_exclude_addons_40 branch 2 times, most recently from 078b4fd to f5d3b25 Compare August 5, 2023 15:33
@hilfazer hilfazer requested a review from a team as a code owner August 5, 2023 15:33
@akien-mga
Copy link
Member

I think this kind of filter should be toggled directly in the Quick Open dialog, and not in editor settings.

So if we think we only want to support excluding results from res://addons, we can add a button for this in that dialog.

If we want to support more arbitrary exclude filters, then we could add a way to hide folders with a right click tooltip, and add a button to toggle showing hidden folders. res://addons could be hidden by default, and more folders could be hidden on a per project basis (saved in project metadata), and unhidden via showing hidden folders (and thus res://addons could be unhidden too when desired). But I don't know if we need that level of flexibility.

@YuriSizov
Copy link
Contributor

YuriSizov commented Aug 7, 2023

There are already requests to exclude addons from other places as well, e.g. godotengine/godot-proposals#7392 for the FileSystem dock filter. Which is why I'm raising my concern.

A project setting with comma-separated wildcards makes sense to me.

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Some minor details, looks good otherwise

editor/editor_quick_open.cpp Outdated Show resolved Hide resolved
editor/editor_quick_open.cpp Outdated Show resolved Hide resolved
editor/editor_quick_open.cpp Outdated Show resolved Hide resolved
@hilfazer hilfazer force-pushed the quick_open_exclude_addons_40 branch 2 times, most recently from 9682a88 to 0e0c6e6 Compare January 20, 2024 12:25
@hilfazer hilfazer force-pushed the quick_open_exclude_addons_40 branch 2 times, most recently from 736f3f6 to a0a00df Compare January 20, 2024 12:33
@AThousandShips
Copy link
Member

Also needs a rebase against the most recent version, it has conflicts

@hilfazer hilfazer force-pushed the quick_open_exclude_addons_40 branch from a0a00df to 4d2cfe3 Compare January 20, 2024 13:22
@YuriSizov YuriSizov self-requested a review January 22, 2024 14:43
@YuriSizov YuriSizov modified the milestones: 4.x, 4.3 Jan 24, 2024
@garf
Copy link

garf commented May 15, 2024

Why not to just allow to exclude folders right-clicking them? Something like that in context menu:

Mark Directory as... > 
    Excluded from Search
    Excluded from Quick Search
    Ignored

This might be scalable and also you can stay in context and show the states directly on the folder

@akien-mga akien-mga modified the milestones: 4.3, 4.x May 28, 2024
@cyda89
Copy link

cyda89 commented Oct 13, 2024

Why not to just allow to exclude folders right-clicking them? Something like that in context menu:

Mark Directory as... > 
    Excluded from Search
    Excluded from Quick Search
    Ignored

This might be scalable and also you can stay in context and show the states directly on the folder

This is excactly what I also had in mind, because that is how it works in other IDEs (e.g. IntelliJ Idea). I would also add an option to exclude from "Find in Files" since i think it would also greatly benefit from this change.

@Repiteo Repiteo modified the milestones: 4.x, 4.4 Oct 16, 2024
@fire
Copy link
Member

fire commented Nov 6, 2024

I added a salvageable tag because there seems to be interest in having this feature.

@Repiteo Repiteo modified the milestones: 4.4, 4.x Nov 6, 2024
@arkology
Copy link
Contributor

As I see this is implemented in #56772
cc @Repiteo

@akien-mga
Copy link
Member

Superseded by #56772. Thanks for the contribution!

@akien-mga akien-mga closed this Nov 15, 2024
@akien-mga akien-mga added archived and removed salvageable cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Nov 15, 2024
@akien-mga akien-mga removed this from the 4.x milestone Nov 15, 2024
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.