Add support for IRM (Insert Replace Mode)#14700
Conversation
ghost
added
Area-VT
|
Here's my approach: https://github.com/microsoft/terminal/compare/dev/lhecker/1947-irm The reason it contains more "fluff" is because I initially tried to implement IRM with proper Unicode support in mind (which means that we need to avoid remeasuring strings) and resulted in a rather complex changeset since it requires a generic text replacement and insertion function. To get it done faster, I removed that code and replaced it with a simpler one that remeasures strings via ...but I failed to implement moving the text attributes until now, whereas your code already (presumably) works flawlessly, while being a rather simple changeset as well. I like it! If you see anything useful in my branch, please feel free to use it. 😊 |
|
I am 100% on board for this, it's so delightfully simple |
|
@lhecker Your branch is doing almost exactly what I hoped you'd be doing. So if we merge this PR first, it would ideal if you could do a follow up at some point that just replaces my I haven't looked at your actual implementation in much detail, but the one key thing that I think is still missing from the And I think we might also need to be able to pass in the wrap flag. It's not obvious what the expected behavior should be with IRM enabled, but it seemed reasonable to me that we should handle that the same as replace mode. I should probably test on some other terminals and see what they do. |
Agreed. I originally looked at how gnome-terminal handled it, and i wasn't terribly surprised by the results. If I recall, it kept the wrap flag on rows up until you got to the end of the line, at which point normal newline/wrapping behavior took over. |
DHowett
left a comment
DHowett
left a comment
There was a problem hiding this comment.
Minor nit, but I'm just blown away by how the foundation you've been laying over the past year has made this look so easy. Leonard as well, of course 🙂
|
|
||
| enum ModeParams : VTInt | ||
| { | ||
| IRM_InsertReplaceMode = ANSIStandardMode(4), |
There was a problem hiding this comment.
I love that my weird "tagged enum" thing didn't turn into YAGNI. I added it because we'd eventually do IRM, and need SM/RM. 😄
There was a problem hiding this comment.
Yeah, it's definitely been useful. I used it for differentiating the ANSI and private DSR operations as well.
Co-authored-by: Dustin L. Howett <[email protected]>
DHowett
added
the
AutoMerge
|
Hello @DHowett! 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: |
ghost
mentioned this pull request
4 participants
This PR add support for the ANSI Insert/Replace mode (
IRM), whichdetermines whether output characters are inserted at the active cursor
position, moving existing content to the right, or whether they should
overwrite the content that is already there.
The implementation is a bit of a hack. When that mode is enabled, it
first measures how many cells the string is expected to occupy, then
scrolls the target line right by that amount before writing out the new
text.
In the longer term it might be better if this was implemented entirely
in the
TextBufferitself, so the scrolling could take place at thesame time as the content was being written.
Validation Steps Performed
I've added a very basic unit test that verifies the mode is working as
expected. But I've also done a lot more manual testing, confirming edge
cases like wide characters, double-width lines, and both with and
without wrapping mode enabled.
Closes #1947