Skip to content

Commit

Permalink
A better fix for #tab-titles-are-too-long (#2373)
Browse files Browse the repository at this point in the history
### User Stories:

1. A user wants to be able to use the executable path as their starting title
    - Does anyone want this?
2. A user wants to be able to set a custom starting title, but have that title be overridable
3. A user wants to be able to set an overridable starting title, different from the profile name
    - Presumably someone will want this
4. A user totally wants to ignore the VT title and use something else
    - This will make more sense in the post [#1320] "Support runtime variables in the custom user title" settings

### Solutions:

1. `name`, `startingTitle`, `tabTitle`
    * a. `name` is only ever used as the profile name.
    * b. If `startingTitle` isn't set, then the executable path is used
    * c. If `startingTitle` is set, it's used as the initial title
    * d. If `tabTitle` is set, it overrides the title from the terminal
    * e. Current users of `tabTitle` need to manually update to the new behavior.
2. `name` as starting title, `tabTitle` as a different starting title
    * a. `name` is used as the starting title and the profile name in the dropdown
    * b. If `tabTitle` is set, we'll use that as the overridable starting title instead.
    * c. In the future, `dynamicTabTitle` or `tabTitleOverride` could be added to support [#1320]
    * d. Current users of `tabTitle` automatically get the new (different!) behavior.
    * e. User Story 1 is impossible
        - Does anyone want the behavior _ever_? Perhaps making that scenario impossible is good?
3. `name` unchanged, `tabTitle` as the starting title
    * a. `name` is only ever used as the profile name.
    * b. If `tabTitle` is set, we'll use that as the overridable starting title.
    * c. In the future, `dynamicTabTitle` or `tabTitleOverride` could be added to support [#1320]
    * d. Current users of `tabTitle` automatically get the new (different!) behavior.
4. `name` as starting title, `tabTitle` as different starting title, `suppressApplicationTitle` Boolean to force it to override
    * a. `name`, `tabTitle` work as in Solution 2.
    * b. When someone wants to be able to statically totally override that title (story 4), they can use `suppressApplicationTitle`
    * c. `suppressApplicationTitle` name is WIP
    * d.  We'll add `suppressApplicationTitle` when someone complains
    * e. If you really want story 1, use `tabTitle: c:\path\to\foo.exe` and `suppressApplicationTitle`.

[#1320]: microsoft/terminal#1320

We've decided to pursue path 4.
  • Loading branch information
donno2048 committed Aug 14, 2019
1 parent 4da3181 commit 38382dd
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 51 deletions.
4 changes: 2 additions & 2 deletions doc/cascadia/SettingsSchema.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Properties listed below are specific to each unique profile.
| `fontSize` | _Required_ | Integer | `10` | Sets the font size. |
| `guid` | _Required_ | String | | Unique identifier of the profile. Written in registry format: `"{00000000-0000-0000-0000-000000000000}"`. |
| `historySize` | _Required_ | Integer | `9001` | The number of lines above the ones displayed in the window you can scroll back to. |
| `name` | _Required_ | String | `PowerShell Core` | Name of the profile. Displays in the dropdown menu. |
| `name` | _Required_ | String | `PowerShell Core` | Name of the profile. Displays in the dropdown menu. <br>Additionally, this value will be used as the "title" to pass to the shell on startup. Some shells (like `bash`) may choose to ignore this initial value, while others (`cmd`, `powershell`) may use this value over the lifetime of the application. This "title" behavior can be overriden by using `tabTitle`. |
| `padding` | _Required_ | String | `0, 0, 0, 0` | Sets the padding around the text within the window. Can have three different formats: `"#"` sets the same padding for all sides, `"#, #"` sets the same padding for left-right and top-bottom, and `"#, #, #, #"` sets the padding individually for left, top, right, and bottom. |
| `snapOnInput` | _Required_ | Boolean | `true` | When set to `true`, the window will scroll to the command input line when typing. When set to `false`, the window will not scroll when you start typing. |
| `startingDirectory` | _Required_ | String | `%USERPROFILE%` | The directory the shell starts in when it is loaded. |
Expand All @@ -44,7 +44,7 @@ Properties listed below are specific to each unique profile.
| `foreground` | Optional | String | | Sets the foreground color of the profile. Overrides `foreground` set in color scheme if `colorscheme` is set. Uses hex color format: `"#rrggbb"`. |
| `icon` | Optional | String | | Image file location of the icon used in the profile. Displays within the tab and the dropdown menu. |
| `scrollbarState` | Optional | String | | Defines the visibility of the scrollbar. Possible values: `"visible"`, `"hidden"` |
| `tabTitle` | Optional | String | | Overrides default title of the tab. |
| `tabTitle` | Optional | String | | If set, will replace the `name` as the title to pass to the shell on startup. Some shells (like `bash`) may choose to ignore this initial value, while others (`cmd`, `powershell`) may use this value over the lifetime of the application. |

## Schemes
Properties listed below are specific to each color scheme. [ColorTool](https://github.com/microsoft/terminal/tree/master/src/tools/ColorTool) is a great tool you can use to create and explore new color schemes. All colors use hex color format.
Expand Down
30 changes: 11 additions & 19 deletions src/cascadia/TerminalApp/App.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -708,25 +708,12 @@ namespace winrt::TerminalApp::implementation
void App::_UpdateTitle(std::shared_ptr<Tab> tab)
{
auto newTabTitle = tab->GetFocusedTitle();
const auto lastFocusedProfileOpt = tab->GetFocusedProfile();
if (lastFocusedProfileOpt.has_value())
{
const auto lastFocusedProfile = lastFocusedProfileOpt.value();
const auto* const matchingProfile = _settings->FindProfile(lastFocusedProfile);

const auto tabTitle = matchingProfile->GetTabTitle();
tab->SetTabText(newTabTitle);

// Checks if tab title has been set in the profile settings and
// updates accordingly.

const auto newActualTitle = tabTitle.empty() ? newTabTitle : tabTitle;

tab->SetTabText(winrt::to_hstring(newActualTitle.data()));
if (_settings->GlobalSettings().GetShowTitleInTitlebar() &&
tab->IsFocused())
{
_titleChangeHandlers(newActualTitle);
}
if (_settings->GlobalSettings().GetShowTitleInTitlebar() &&
tab->IsFocused())
{
_titleChangeHandlers(newTabTitle);
}
}

Expand Down Expand Up @@ -1456,7 +1443,12 @@ namespace winrt::TerminalApp::implementation
}
else
{
connection = TerminalConnection::ConhostConnection(settings.Commandline(), settings.StartingDirectory(), settings.InitialRows(), settings.InitialCols(), winrt::guid());
connection = TerminalConnection::ConhostConnection(settings.Commandline(),
settings.StartingDirectory(),
settings.StartingTitle(),
settings.InitialRows(),
settings.InitialCols(),
winrt::guid());
}

TraceLoggingWrite(
Expand Down
24 changes: 4 additions & 20 deletions src/cascadia/TerminalApp/Profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ TerminalSettings Profile::CreateTerminalSettings(const std::vector<ColorScheme>&
terminalSettings.StartingDirectory(winrt::to_hstring(evaluatedDirectory.c_str()));
}

// GH#2373: Use the tabTitle as the starting title if it exists, otherwise
// use the profile name
terminalSettings.StartingTitle(_tabTitle ? _tabTitle.value() : _name);

if (_schemeName)
{
const ColorScheme* const matchingScheme = _FindScheme(schemes, _schemeName.value());
Expand Down Expand Up @@ -594,26 +598,6 @@ std::wstring_view Profile::GetName() const noexcept
return _name;
}

// Method Description:
// - Returns true if profile's custom tab title is set, if one is set. Otherwise returns false.
// Return Value:
// - true if this profile's custom tab title is set. Otherwise returns false.
bool Profile::HasTabTitle() const noexcept
{
return _tabTitle.has_value();
}

// Method Description:
// - Returns the custom tab title, if one is set. Otherwise returns the empty string.
// Return Value:
// - this profile's custom tab title, if one is set. Otherwise returns the empty string.
std::wstring_view Profile::GetTabTitle() const noexcept
{
return HasTabTitle() ?
std::wstring_view{ _tabTitle.value().c_str(), _tabTitle.value().size() } :
std::wstring_view{ L"", 0 };
}

bool Profile::HasConnectionType() const noexcept
{
return _connectionType.has_value();
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/TerminalApp/Profile.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ class TerminalApp::Profile final

GUID GetGuid() const noexcept;
std::wstring_view GetName() const noexcept;
bool HasTabTitle() const noexcept;
std::wstring_view GetTabTitle() const noexcept;
bool HasConnectionType() const noexcept;
GUID GetConnectionType() const noexcept;

Expand Down
17 changes: 17 additions & 0 deletions src/cascadia/TerminalConnection/ConhostConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
ConhostConnection::ConhostConnection(const hstring& commandline,
const hstring& startingDirectory,
const hstring& startingTitle,
const uint32_t initialRows,
const uint32_t initialCols,
const guid& initialGuid) :
_initialRows{ initialRows },
_initialCols{ initialCols },
_commandline{ commandline },
_startingDirectory{ startingDirectory },
_startingTitle{ startingTitle },
_guid{ initialGuid }
{
if (_guid == guid{})
Expand Down Expand Up @@ -90,6 +92,20 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation

THROW_LAST_ERROR_IF_NULL(_hOutputThread);

STARTUPINFO si = { 0 };
si.cb = sizeof(STARTUPINFOW);

// If we have a startingTitle, create a mutable character buffer to add
// it to the STARTUPINFO.
std::unique_ptr<wchar_t[]> mutableTitle{ nullptr };
if (!_startingTitle.empty())
{
mutableTitle = std::make_unique<wchar_t[]>(_startingTitle.size() + 1);
THROW_IF_NULL_ALLOC(mutableTitle);
THROW_IF_FAILED(StringCchCopy(mutableTitle.get(), _startingTitle.size() + 1, _startingTitle.c_str()));
si.lpTitle = mutableTitle.get();
}

THROW_IF_FAILED(
CreateConPty(cmdline,
startingDirectory,
Expand All @@ -100,6 +116,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
&_signalPipe,
&_piConhost,
0,
si,
extraEnvVars));

_connected = true;
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalConnection/ConhostConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{
struct ConhostConnection : ConhostConnectionT<ConhostConnection>
{
ConhostConnection(const hstring& cmdline, const hstring& startingDirectory, const uint32_t rows, const uint32_t cols, const guid& guid);
ConhostConnection(const hstring& cmdline, const hstring& startingDirectory, const hstring& startingTitle, const uint32_t rows, const uint32_t cols, const guid& guid);

winrt::event_token TerminalOutput(TerminalConnection::TerminalOutputEventArgs const& handler);
void TerminalOutput(winrt::event_token const& token) noexcept;
Expand All @@ -30,6 +30,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
uint32_t _initialCols{};
hstring _commandline;
hstring _startingDirectory;
hstring _startingTitle;
guid _guid{}; // A unique session identifier for connected client

bool _connected{};
Expand Down
5 changes: 2 additions & 3 deletions src/cascadia/TerminalConnection/ConhostConnection.idl
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ import "ITerminalConnection.idl";

namespace Microsoft.Terminal.TerminalConnection
{
[default_interface]
runtimeclass ConhostConnection : ITerminalConnection
[default_interface] runtimeclass ConhostConnection : ITerminalConnection
{
ConhostConnection(String cmdline, String startingDirectory, UInt32 rows, UInt32 columns, Guid guid);
ConhostConnection(String cmdline, String startingDirectory, String startingTitle, UInt32 rows, UInt32 columns, Guid guid);

Guid Guid { get; };
};
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettings/IControlSettings.idl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace Microsoft.Terminal.Settings
Boolean UseAcrylic;
Boolean CloseOnExit;
Double TintOpacity;
ScrollbarState ScrollState;
ScrollbarState ScrollState;

String FontFace;
Int32 FontSize;
Expand All @@ -32,6 +32,7 @@ namespace Microsoft.Terminal.Settings

String Commandline;
String StartingDirectory;
String StartingTitle;
String EnvironmentVariables;

String BackgroundImage;
Expand Down
10 changes: 10 additions & 0 deletions src/cascadia/TerminalSettings/TerminalSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,16 @@ namespace winrt::Microsoft::Terminal::Settings::implementation
_startingDir = value;
}

hstring TerminalSettings::StartingTitle()
{
return _startingTitle;
}

void TerminalSettings::StartingTitle(hstring const& value)
{
_startingTitle = value;
}

hstring TerminalSettings::EnvironmentVariables()
{
return _envVars;
Expand Down
4 changes: 4 additions & 0 deletions src/cascadia/TerminalSettings/terminalsettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ namespace winrt::Microsoft::Terminal::Settings::implementation
hstring StartingDirectory();
void StartingDirectory(hstring const& value);

hstring StartingTitle();
void StartingTitle(hstring const& value);

hstring EnvironmentVariables();
void EnvironmentVariables(hstring const& value);

Expand Down Expand Up @@ -115,6 +118,7 @@ namespace winrt::Microsoft::Terminal::Settings::implementation
winrt::Windows::UI::Xaml::VerticalAlignment _backgroundImageVerticalAlignment;
hstring _commandline;
hstring _startingDir;
hstring _startingTitle;
hstring _envVars;
Settings::IKeyBindings _keyBindings;
Settings::ScrollbarState _scrollbarState;
Expand Down
6 changes: 5 additions & 1 deletion src/inc/conpty-universal.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,9 @@ bool SignalResizeWindow(const HANDLE hSignal,
// - hSignal: A handle to the pipe for writing signal messages to the pty.
// - piPty: The PROCESS_INFORMATION of the pty process. NOTE: This is *not* the
// PROCESS_INFORMATION of the process that's created as a result the cmdline.
// - startupInfo : A STARTUPINFO struct to use as additional information to pass
// into the created conhost process. Conhost may pass some of the properties
// in this struct to its child process, notably, the lpTitle.
// - extraEnvVars : A map of pairs of (Name, Value) representing additional
// environment variable strings and values to be set in the client process
// environment. May override any already present in parent process.
Expand All @@ -221,6 +224,7 @@ bool SignalResizeWindow(const HANDLE hSignal,
HANDLE* const hSignal,
PROCESS_INFORMATION* const piPty,
DWORD dwCreationFlags = 0,
const STARTUPINFO startupInfo = { 0 },
const EnvironmentVariableMapW& extraEnvVars = {}) noexcept
{
// Create some anon pipes so we can pass handles down and into the console.
Expand Down Expand Up @@ -268,7 +272,7 @@ bool SignalResizeWindow(const HANDLE hSignal,
conhostCmdline += L" -- ";
conhostCmdline += cmdline;

STARTUPINFO si = { 0 };
STARTUPINFO si = startupInfo;
si.cb = sizeof(STARTUPINFOW);
si.hStdInput = inPipeConhostSide;
si.hStdOutput = outPipeConhostSide;
Expand Down
8 changes: 6 additions & 2 deletions src/server/Entrypoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,14 @@
HostStartupInfo.cb = sizeof(STARTUPINFO);
GetStartupInfoW(&HostStartupInfo);

// If we were started with Title is Link Name, then pass the flag and the link name down to the child.
// Pass the title we were started with down to our child process.
// Conhost itself absolutely doesn't care about this value, but the
// child might.
StartupInformation.StartupInfo.lpTitle = HostStartupInfo.lpTitle;
// If we were started with Title is Link Name, then pass the flag
// down to the child. (the link name was already passed down above)
if (WI_IsFlagSet(HostStartupInfo.dwFlags, STARTF_TITLEISLINKNAME))
{
StartupInformation.StartupInfo.lpTitle = HostStartupInfo.lpTitle;
StartupInformation.StartupInfo.dwFlags |= STARTF_TITLEISLINKNAME;
}
}
Expand Down

0 comments on commit 38382dd

Please sign in to comment.