Implement the FTCS_PROMPT sequence for marking the start of the prompt#13163
Implement the FTCS_PROMPT sequence for marking the start of the prompt#1316341 commits merged intomainfrom
Conversation
…erimental-marks # Conflicts: # src/terminal/adapter/ITermDispatch.hpp # src/terminal/adapter/adaptDispatch.hpp # src/terminal/adapter/termDispatch.hpp
This comment was marked as resolved.
This comment was marked as resolved.
| return false; | ||
| } | ||
|
|
||
| const auto action{ parts[0] }; |
There was a problem hiding this comment.
I don't believe this code requires a string copy. This avoids one:
| const auto action{ parts[0] }; | |
| const auto& action = parts[0]; |
There was a problem hiding this comment.
I see you prefer he not use the uniform initialization syntax (since you switched it to =) 😉
| const auto action{ parts[0] }; | |
| const auto& action{ parts[0] }; |
I had to read the rules about const auto& locals and how they extend lifetime -- I still think it's a bad pattern to encourage, but we use it all over the place \_(shrug)_/
There was a problem hiding this comment.
I see you prefer he not use the uniform initialization syntax (since you switched it to
=) 😉
The rest of the file uses classic = assignments. I like consistency. 👉👈
Of course you probably already figured out my deepest secret that - entirely unrelated to this - I personally kinda dislike the uniform initialization syntax. 😅 This is what I write:
auto bar = bar_producer(); // good
auto bar{ bar_producer() }; // "bad"
Foo foo{ bar }; // goodIsn't the uniform initialization syntax nothing but a bolted on language hack, because people were afraid of breaking existing code by making round brackets better? I use it because it's very useful, but would you write this if it was an equivalent thing?
auto bar(bar_producer());I wouldn't and so I don't use curly brackets either, because I consider curly brackets stricter round brackets. In my mind I'm not constructing an object there, I'm assigning an unnamed rvalue a name via variable assignment. And so I write auto result = foo().
I had to read the rules about
const auto&locals and how they extend lifetime -- I still think it's a bad pattern to encourage, but we use it all over the place \_(shrug)_/
I think about it differently: Here we "bind to the memory address" of the first item in an array.
There was a problem hiding this comment.
ultimately, Audit will complain:
error C26445: Do not assign gsl::span or std::string_view to a reference. They are cheap to construct and are not owners of the underlying data.
so, const auto action = ... it is
There was a problem hiding this comment.
error C26445: Do not assign gsl::span or std::string_view to a reference.
But passing it as a by-reference argument is fine? smh
There was a problem hiding this comment.
I guess the underlying point is: this is not a string copy, it's a string_view copy ;)
…/11000-FTCS-simple
|
My nits are just comment changes? Huh. |
zadjii-msft
added
Area-VT
|
Hello @zadjii-msft! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
|
🎉 Handy links: |
|
Quick note since this is currently the Windows Terminal bug-tracker example for adding There also seems to be an extraneous Edit: I did eventually get it working ( |
5 participants
Implements the FTCS_PROMPT sequence,
OSC 133 ; A ST. In this PR, it's just used to set a simple Prompt mark on the current line, in the same way that the iTerm2 sequence works.There's rumination in #11000 on how to implement the rest of the FTCS sequences.
This is broken into its own PR at the moment. Quoth j4james:
This part of the plumbing is super easy, so I thought it would be valuable to add regardless if we get to the whole of FTCS in 1.15.
$PROFILE: