Skip to content

Remove the need to have a display name for built-in action overrides #107487

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

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

Conversation

roughbits01
Copy link
Contributor

@roughbits01 roughbits01 commented Jun 13, 2025

This PR

  • Adds missing built-in action display names
  • Fixes extra space in "ui_swap_input_direction "
  • Removes the need to add a display name for built-in action overrides

@KoBeWi
Copy link
Member

KoBeWi commented Jul 28, 2025

The code looks good, but where are these names displayed? 🤔

@Mickeon
Copy link
Member

Mickeon commented Jul 29, 2025

The new strings added in this PR are not referenced anywhere, directly. I tried justifying their addition but I can't really find a reason as to why they're here. It's seemingly arbitrary.

@roughbits01
Copy link
Contributor Author

roughbits01 commented Jul 29, 2025

The new strings added in this PR are not referenced anywhere, directly.

They are referenced in the code like any other built-in with an existing description. Take for example ui_text_completion_query which already has a description vs ui_text_completion_accept and ui_text_completion_replace for which the description is added in this PR. Is ui_text_completion_query not referenced as well?

I tried justifying their addition but I can't really find a reason as to why they're here. It's seemingly arbitrary.

What I did is making sure we have a description for all builtin actions. The goal was consistency rather than an arbitrary change.

The code looks good, but where are these names displayed? 🤔

That's a good question, and after checking I didn't find any usage in the editor. If the description is not used then maybe we should get rid of it all together. But then it seems we are still maintaining that list #107471 #87883 #76829

@KoBeWi
Copy link
Member

KoBeWi commented Jul 29, 2025

Some action names are displayed in context menus in LineEdit/TextEdit.

@roughbits01
Copy link
Contributor Author

Also a good place to have the description of the built-in shown is the Input Map tab in the project settings (on hover for example). But that needs a PR.
image

@roughbits01
Copy link
Contributor Author

@KoBeWi @Mickeon Do you think these two changes are still worth it?

  • Fixes extra space in "ui_swap_input_direction "
  • Removes the need to add a display name for built-in action overrides

If so I can remove the added strings.

@KoBeWi
Copy link
Member

KoBeWi commented Jul 29, 2025

Yeah these 2 changes are fine.

@roughbits01 roughbits01 changed the title Add missing built-in action display names in InputMap Removes the need to have a display name for built-in action overrides Jul 29, 2025
@AThousandShips AThousandShips changed the title Removes the need to have a display name for built-in action overrides Remove the need to have a display name for built-in action overrides Jul 29, 2025
@Repiteo Repiteo modified the milestones: 4.5, 4.6 Jul 29, 2025
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.

5 participants