Move commandline->title promotion into TerminalSettings #11029
Conversation
It was insufficient to only promote commandline components to titles during commandline parsing, because we also have a whole complement of actions that contain NewTerminalArgs. The tests caught me out a little too late (sorry!). I decided it was better move promotion down to TerminalSettings. Fixes #6776.
ghost
added
Area-Commandline
| const auto start{ til::at(commandLine, 0) == L'"' ? 1 : 0 }; | ||
| const auto terminator{ commandLine.find_first_of(start ? L'"' : L' ', start) }; // look past the first character if it starts with " | ||
| // We have to take a copy here; winrt::param::hstring requires a null-terminated string |
There was a problem hiding this comment.
this is rough but it works. doing a substr with npos-x later is safe (confirmed by docs)
There was a problem hiding this comment.
if it yields a title of nothing, we already have a weird commandline. If the title is too long, oh well
There was a problem hiding this comment.
Are you sure about this? winrt::param::hstring has an explicit std::wstring_view constructor and appears to work just fine with it.
There was a problem hiding this comment.
Dead sure. This blog post just came out today to reinforce: https://devblogs.microsoft.com/oldnewthing/20210823-00/?p=105598
ghost
requested review from
miniksa,
carlos-zamora,
leonMSFT,
PankajBhojwani and
lhecker
|
Also, while it's less efficient, I think we should introduce template<typename T, typename Traits>
constexpr std::basic_string_view<T, Traits> trim_prefix(const std::basic_string_view<T, Traits>& str, const std::basic_string_view<T, Traits>& prefix) noexcept
{
#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead
return starts_with(str, prefix) ? std::basic_string_view<T, Traits>{ str.data() + prefix.size(), str.size() - prefix.size() } : str;
}
constexpr std::string_view trim_prefix(const std::string_view& str, const std::string_view& prefix) noexcept
{
return trim_prefix<>(str, prefix);
}
constexpr std::wstring_view trim_prefix(const std::wstring_view& str, const std::wstring_view& prefix) noexcept
{
return trim_prefix<>(str, prefix);
}
template<typename T, typename Traits>
constexpr std::basic_string_view<T, Traits> trim_suffix(const std::basic_string_view<T, Traits>& str, const std::basic_string_view<T, Traits>& suffix) noexcept
{
#pragma warning(suppress : 26481) // Don't use pointer arithmetic. Use span instead
return ends_with(str, suffix) ? std::basic_string_view<T, Traits>{ str.data(), str.size() - prefix.size() } : str;
}
constexpr std::string_view trim_suffix(const std::string_view& str, const std::string_view& suffix) noexcept
{
return trim_prefix<>(str, suffix);
}
constexpr std::wstring_view trim_suffix(const std::wstring_view& str, const std::wstring_view& suffix) noexcept
{
return trim_prefix<>(str, suffix);
}
// ...
const auto firstComponent = til::trim_suffix(til::trim_prefix(commandLine, L'"'), L'"'); |
|
That doesn't do the same thing as this, though. This converts |
|
@msftbot merge this in 2 minutes |
ghost
added
the
AutoMerge
|
Hello @DHowett! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
|
🎉 Handy links: |
ghost
mentioned this pull request
It was insufficient to only promote commandline components to titles
during commandline parsing, because we also have a whole complement of
actions that contain NewTerminalArgs. The tests caught me out a little
too late (sorry!). I decided it was better move promotion down to
TerminalSettings.
Fixes #6776
Re-implements #10998