fskorgen opened a new issue, #8230: URL: https://github.com/apache/hop/issues/8230
### Apache Hop version? 2.19 ### Java version? 21 ### Operating system Windows ### What happened? Three leftovers from the terminal panel's move from the View menu to Tools. None of them breaks anything today, which is exactly why they are worth removing before something starts using them. ### 1. Two menu ids are the same string `HopGui.java:206-208`: ```java public static final String ID_MAIN_MENU_VIEW_FULL_SCREEN = "25010-menu-view-full-screen"; public static final String ID_MAIN_MENU_VIEW_TERMINAL = "25010-menu-view-terminal"; public static final String ID_MAIN_MENU_VIEW_NEW_TERMINAL = "25020-menu-view-new-terminal"; ``` The numeric prefix is the sort key **and** part of the registry key: `GuiRegistry.addGuiMenuItem` puts items in a `Map<String, GuiMenuItem>` keyed by id, and `GuiMenuWidgets` sorts children by id to get a stable menu order. `25010` is therefore claimed twice. Two items whose keys differ only after the shared prefix sort next to each other by accident rather than by intent, and any code that keys off the prefix alone sees a clash. `ID_MAIN_MENU_VIEW_NEW_TERMINAL` takes `25020`, which is the next free slot in the View menu — so a new View item added there today would collide with a constant nothing uses. ### 2. Neither terminal constant is referenced Both are `public static final` and referenced by nothing in the repository. The menu items that replaced them live in `HopGuiBottomDock`, with their own ids: ```java public static final String ID_MAIN_MENU_TOOLS_TERMINAL = "40010-menu-tools-terminal"; public static final String ID_MAIN_MENU_TOOLS_NEW_TERMINAL = "40020-menu-tools-new-terminal"; ``` Because they are `public`, they are also part of the API surface for GUI plugins — an out-of-tree plugin that parents a menu item to `ID_MAIN_MENU_VIEW_TERMINAL` gets a menu item under an id that belongs to full screen. ### 3. `PropsUi.USE_ADVANCED_TERMINAL` is a private constant nobody reads `PropsUi.java:94`: ```java private static final String USE_ADVANCED_TERMINAL = "UseAdvancedTerminal"; ``` No getter, no setter, no reference. A property key that is never read from or written to the properties file. ### Steps to reproduce Static, not runtime: `grep -rn ID_MAIN_MENU_VIEW_TERMINAL ui/src` and `grep -rn USE_ADVANCED_TERMINAL ui/src` each return their declaration and nothing else. ### Expected behaviour Either the constants are used, or they are gone. If the View entries are meant to come back, the terminal one needs an id of its own rather than full screen's. ### Suggested fix Delete all three declarations. Nothing references them, so nothing else changes. Happy to open a PR. ### Note Found while switching the terminal off by seeding `GuiRegistry.getDisabledGuiElements()`, which meant walking every id the terminal surfaces use. The Tools ids are correct and in use; only the View pair and the `PropsUi` key are stranded. ### Issue Priority Priority: 3 ### Issue Component Component: Hop Gui -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
