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

Add support for displaying the version with wt --version #5501

Merged
3 commits merged into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.

#include "pch.h"
#include "AppLogic.h"
#include "AppCommandlineArgs.h"
#include "ActionArgs.h"
#include <LibraryResources.h>
Expand Down Expand Up @@ -140,6 +141,11 @@ int AppCommandlineArgs::_handleExit(const CLI::App& command, const CLI::Error& e
{
_exitMessage = err.str();
}

// We're displaying an error message - we should always exit instead of
// actually starting the Terminal.
_shouldExitEarly = true;

return result;
}

Expand All @@ -151,6 +157,22 @@ int AppCommandlineArgs::_handleExit(const CLI::App& command, const CLI::Error& e
// - <none>
void AppCommandlineArgs::_buildParser()
{
auto versionCallback = [this](int64_t /*count*/) {
if (const auto appLogic{ winrt::TerminalApp::implementation::AppLogic::Current() })
{
// Set our message to display the application name and the current version.
_exitMessage = fmt::format("{0}\n{1}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yyyyaaaasssss

til::u16u8(appLogic->ApplicationDisplayName()),
til::u16u8(appLogic->ApplicationVersion()));
// Theoretically, we don't need to exit now, since this isn't really
// an error case. However, in practice, it feels weird to have `wt
// -v` open a new tab, and makes enough sense that `wt -v ;
// split-pane` (or whatever) just displays the version and exits.
_shouldExitEarly = true;
}
};
_app.add_flag_function("-v,--version", versionCallback, RS_A(L"CmdVersionDesc"));

_buildNewTabParser();
_buildSplitPaneParser();
_buildFocusTabParser();
Expand Down Expand Up @@ -540,6 +562,19 @@ const std::string& AppCommandlineArgs::GetExitMessage()
return _exitMessage;
}

// Method Description:
// - Returns true if we should exit the application before even starting the
// window. We might want to do this if we're displaying an error message or
// the version string, or if we want to open the settings file.
// Arguments:
// - <none>
// Return Value:
// - true iff we should exit the application before even starting the window
bool AppCommandlineArgs::ShouldExitEarly() const noexcept
{
return _shouldExitEarly;
}

// Method Description:
// - Ensure that the first command in our list of actions is a NewTab action.
// This makes sure that if the user passes a commandline like "wt split-pane
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class TerminalApp::AppCommandlineArgs final
void ValidateStartupCommands();
std::deque<winrt::TerminalApp::ActionAndArgs>& GetStartupActions();
const std::string& GetExitMessage();
bool ShouldExitEarly() const noexcept;

private:
static const std::wregex _commandDelimiterRegex;
Expand Down Expand Up @@ -79,6 +80,7 @@ class TerminalApp::AppCommandlineArgs final

std::deque<winrt::TerminalApp::ActionAndArgs> _startupActions;
std::string _exitMessage;
bool _shouldExitEarly{ false };

winrt::TerminalApp::NewTerminalArgs _getNewTerminalArgs(NewTerminalSubcommand& subcommand);
void _addNewTerminalArgs(NewTerminalSubcommand& subcommand);
Expand Down
13 changes: 11 additions & 2 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -880,15 +880,24 @@ namespace winrt::TerminalApp::implementation
return 0;
}

winrt::hstring AppLogic::EarlyExitMessage()
winrt::hstring AppLogic::ParseCommandlineMessage()
{
if (_root)
{
return _root->EarlyExitMessage();
return _root->ParseCommandlineMessage();
}
return { L"" };
}

bool AppLogic::ShouldExitEarly()
{
if (_root)
{
return _root->ShouldExitEarly();
}
return false;
}

winrt::hstring AppLogic::ApplicationDisplayName() const
{
try
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ namespace winrt::TerminalApp::implementation
[[nodiscard]] std::shared_ptr<::TerminalApp::CascadiaSettings> GetSettings() const noexcept;

int32_t SetStartupCommandline(array_view<const winrt::hstring> actions);
winrt::hstring EarlyExitMessage();
winrt::hstring ParseCommandlineMessage();
bool ShouldExitEarly();

winrt::hstring ApplicationDisplayName() const;
winrt::hstring ApplicationVersion() const;
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ namespace TerminalApp
MaximizedMode,
};

[default_interface] runtimeclass AppLogic: IF7Listener {
[default_interface] runtimeclass AppLogic : IF7Listener
{
AppLogic();

// For your own sanity, it's better to do setup outside the ctor.
Expand All @@ -28,7 +29,8 @@ namespace TerminalApp
Boolean IsElevated();

Int32 SetStartupCommandline(String[] commands);
String EarlyExitMessage { get; };
String ParseCommandlineMessage { get; };
Boolean ShouldExitEarly { get; };

void LoadSettings();
Windows.UI.Xaml.UIElement GetRoot();
Expand Down
57 changes: 30 additions & 27 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema

Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes

The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.

Example:

... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple

There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the

Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not

The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can

Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.

mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -221,6 +221,9 @@
<value>Open in the given directory instead of the profile's set "startingDirectory"</value>
<comment>{Locked="\"startingDirectory\""}</comment>
</data>
<data name="CmdVersionDesc" xml:space="preserve">
<value>Display the application version</value>
</data>
<data name="NewTabSplitButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.HelpText" xml:space="preserve">
<value>Press the button to open a new terminal tab with your default profile. Open the flyout to select which profile you want to open.</value>
</data>
Expand Down
17 changes: 16 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1732,16 +1732,31 @@ namespace winrt::TerminalApp::implementation
// message. If there were no errors, this message will be blank.
// - If the user requested help on any command (using --help), this will
// contain the help message.
// - If the user requested the version number (using --version), this will
// contain the version string.
// Arguments:
// - <none>
// Return Value:
// - the help text or error message for the provided commandline, if one
// exists, otherwise the empty string.
winrt::hstring TerminalPage::EarlyExitMessage()
winrt::hstring TerminalPage::ParseCommandlineMessage()
{
return winrt::to_hstring(_appArgs.GetExitMessage());
}

// Method Description:
// - Returns true if we should exit the application before even starting the
// window. We might want to do this if we're displaying an error message or
// the version string, or if we want to open the settings file.
// Arguments:
// - <none>
// Return Value:
// - true iff we should exit the application before even starting the window
bool TerminalPage::ShouldExitEarly()
{
return _appArgs.ShouldExitEarly();
}

// Method Description:
// - Returns a com_ptr to the implementation type of the tab at the given index
// Arguments:
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ namespace winrt::TerminalApp::implementation
void CloseWindow();

int32_t SetStartupCommandline(winrt::array_view<const hstring> args);
winrt::hstring EarlyExitMessage();
winrt::hstring ParseCommandlineMessage();
bool ShouldExitEarly();

// -------------------------------- WinRT Events ---------------------------------
DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(TitleChanged, _titleChangeHandlers, winrt::Windows::Foundation::IInspectable, winrt::hstring);
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.idl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ namespace TerminalApp
TerminalPage();

Int32 SetStartupCommandline(String[] commands);
String EarlyExitMessage { get; };
String ParseCommandlineMessage { get; };
Boolean ShouldExitEarly { get; };

// XAML bound properties
String ApplicationDisplayName { get; };
Expand Down
7 changes: 5 additions & 2 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void AppHost::_HandleCommandlineArgs()
}

const auto result = _logic.SetStartupCommandline({ args });
const auto message = _logic.EarlyExitMessage();
const auto message = _logic.ParseCommandlineMessage();
if (!message.empty())
{
const auto displayHelp = result == 0;
Expand All @@ -115,7 +115,10 @@ void AppHost::_HandleCommandlineArgs()
GetStringResource(messageTitle).data(),
MB_OK | messageIcon);

ExitProcess(result);
if (_logic.ShouldExitEarly())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we're still covered in the version case by the message being non-empty -- we could simplify this changeset more if there's no compelling reason to introduce ShoudlExitEarly up through the stack. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So sure, we don't need it in this PR anymore. Originally I was only going to exit early if there were no subcommands specified after a --version.

Now, I'm thinking about using it in the future for open-settings - if there's no subcommands other than open-settings, just open the file and exit. I don't feel strongly about keeping it around for now, but I will probably be using it in the future so ¯\_(ツ)_/¯

{
ExitProcess(result);
}
}
}
}
Expand Down