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

Ability to set terminal tab name programmatically and not to reset it with ANSI command #494

Closed
mmx85 opened this issue Aug 7, 2023 Discussed in #492 · 4 comments · Fixed by #508
Closed

Ability to set terminal tab name programmatically and not to reset it with ANSI command #494

mmx85 opened this issue Aug 7, 2023 Discussed in #492 · 4 comments · Fixed by #508
Labels
terminal The TM Terminal collection of features in CDT
Milestone

Comments

@mmx85
Copy link
Contributor

mmx85 commented Aug 7, 2023

Discussed in #492

Originally posted by mmx85 August 4, 2023
I have the code that setups terminal and opens it with custom tab name after some user actions. It looks like this:

HashMap<String, Object> params = new HashMap<String, Object>();
params.put(ITerminalsConnectorConstants.PROP_TITLE, "some name"));
params.put(ITerminalsConnectorConstants.PROP_DELEGATE_ID, delegateId);
params.put(ITerminalsConnectorConstants.PROP_PROCESS_PATH, bashShellExecutablePath);
params.put(ITerminalsConnectorConstants.PROP_PROCESS_WORKING_DIR, workingDir);
...
TerminalServiceFactory.getService().openConsole(params, null);

It worked well for me with CDT 10.0.

After updating to CDT 10.2 and higher the code above sets the correct name but VT100Emulator.processNewText() overrides it reading a startup sequence that Bash outputs with ANSI command to change terminal title to current working directory value. When VT100Emulator reads this command it overrides terminal tab title with current working directory value calling: processNewText()->processAnsiOsCommand()->terminal.setTerminalTitle(ansiOsCommand.substring(2)) in VT100Emulator class.

This regression is cased by the following changes: https://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=fe003208544902646096db658f63d2038a80d40a

I need to have ability to keep the value that is set with ITerminalsConnectorConstants.PROP_TITLE parameter regardless what is set with "\e]0;...\u0007" ANSI command and when but this does not meant that feature that caused regression should be reverted or something like this.

I propose to add a ITerminalsConnectorConstants.PROP_TITLE_UPDATE constant with options:

  1. PROP_TITLE_UPDATE_API (to enable title change from menu and with value set to ITerminalsConnectorConstants.PROP_TITLE parameter but disable ability to change title with ANSI command "\e]0;...\u0007") and
  2. PROP_TITLE_UPDATE_MIXED (keep current behavior).
@jonahgraham
Copy link
Member

@mmx85 your proposal sounds great and I look forward to reviewing and merging such a change.

jonahgraham pushed a commit to mmx85/cdt that referenced this issue Aug 18, 2023
When a more complete implementation of ANSI Escape sequence for
renaming terminal titles was added in
[CDT 10.2](https://github.com/eclipse-cdt/cdt/blob/main/NewAndNoteworthy/CDT-10.2.md#rename-terminal-tab)
it caused a regression in use cases where extenders of the terminal
wanted to retain control of the terminal's title.

This commit adds a new flag that will prevent the title of the
terminal tab from being updated from ANSI escape sequences.

Fixes eclipse-cdt#494
jonahgraham pushed a commit that referenced this issue Aug 19, 2023
When a more complete implementation of ANSI Escape sequence for
renaming terminal titles was added in
[CDT 10.2](https://github.com/eclipse-cdt/cdt/blob/main/NewAndNoteworthy/CDT-10.2.md#rename-terminal-tab)
it caused a regression in use cases where extenders of the terminal
wanted to retain control of the terminal's title.

This commit adds a new flag that will prevent the title of the
terminal tab from being updated from ANSI escape sequences.

Fixes #494
@jonahgraham
Copy link
Member

Thank you @mmx85 for this improvement.

@jonahgraham jonahgraham added the terminal The TM Terminal collection of features in CDT label Aug 19, 2023
@jonahgraham jonahgraham added this to the 11.3.0 milestone Aug 19, 2023
davmac314 pushed a commit to davmac314/cdt that referenced this issue Sep 24, 2023
When a more complete implementation of ANSI Escape sequence for
renaming terminal titles was added in
[CDT 10.2](https://github.com/eclipse-cdt/cdt/blob/main/NewAndNoteworthy/CDT-10.2.md#rename-terminal-tab)
it caused a regression in use cases where extenders of the terminal
wanted to retain control of the terminal's title.

This commit adds a new flag that will prevent the title of the
terminal tab from being updated from ANSI escape sequences.

Fixes eclipse-cdt#494
@hloehnert
Copy link

After updating I get following exception when opening a terminal in eclipse, which I think is related to this issue:
java.lang.NullPointerException: Cannot invoke "java.util.Map.containsKey(Object)" because "properties" is null
at org.eclipse.tm.terminal.view.ui.tabs.TabTerminalListener.lambda$1(TabTerminalListener.java:130)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4047)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3663)
at org.eclipse.tm.internal.terminal.emulator.VT100TerminalControl.waitForConnect(VT100TerminalControl.java:468)
at org.eclipse.tm.internal.terminal.emulator.VT100TerminalControl.connectTerminal(VT100TerminalControl.java:423)
at org.eclipse.tm.terminal.view.ui.tabs.TabFolderManager.createTabItem(TabFolderManager.java:330)
at org.eclipse.tm.terminal.view.ui.manager.ConsoleManager.openConsole(ConsoleManager.java:489)
at org.eclipse.tm.terminal.view.ui.services.TerminalService$2.doRun(TerminalService.java:304)
at org.eclipse.tm.terminal.view.ui.services.TerminalService$2.lambda$0(TerminalService.java:274)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4047)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3663)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1155)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:342)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1046)
at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:645)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:342)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:552)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:171)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:136)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:402)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:578)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:651)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:588)
at org.eclipse.equinox.launcher.Main.run(Main.java:1459)

@jonahgraham
Copy link
Member

Thanks @hloehnert for raising this and identifying the likely point it regressed from. To make sure it doesn't get lost I started a new issue #617

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
terminal The TM Terminal collection of features in CDT
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants