Skip to content

[Bug]: Dead terminal constants in HopGui, and an id that collides with full screen #8230

Description

@fskorgen

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:

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:

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:

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions