Conversation
ghost
added
Area-UserInterface
lovef
force-pushed
the
trim-paste
branch
2 times, most recently
from
18bea3b to
6454f73
Compare
ghost
requested review from
DHowett,
PankajBhojwani,
carlos-zamora,
leonMSFT,
miniksa and
zadjii-msft
|
@msftbot don't merge this before friday at 7pm |
|
Hello @zadjii-msft! I think you told me that you want to delay the approval for a certain amount of time, but I am not confident that I have understood you correctly. Please try rephrasing your instruction to me. |
|
@msftbot merge this friday at 7pm |
|
Hello @zadjii-msft! I think you told me that you would like to reset custom auto-merge settings, but I am not confident that I have understood you correctly. Please try rephrasing your instruction to me. |
zadjii-msft
left a comment
zadjii-msft
left a comment
There was a problem hiding this comment.
Thanks for whipping this up!
| INHERITABLE_SETTING(Model::GlobalAppSettings, winrt::Microsoft::Terminal::Control::CopyFormat, CopyFormatting, 0); | ||
| INHERITABLE_SETTING(Model::GlobalAppSettings, bool, WarnAboutLargePaste, true); | ||
| INHERITABLE_SETTING(Model::GlobalAppSettings, bool, WarnAboutMultiLinePaste, true); | ||
| INHERITABLE_SETTING(Model::GlobalAppSettings, bool, TrimPaste, false); |
There was a problem hiding this comment.
| INHERITABLE_SETTING(Model::GlobalAppSettings, bool, TrimPaste, false); | |
| INHERITABLE_SETTING(Model::GlobalAppSettings, bool, TrimPaste, true); |
Warm take: enable this by default. It's a sensible QoL improvement. I'm gonna give the team till Friday to NAK me on this one.
@lovef If it's not too much trouble, could you add this to defaults.json as well? (mostly just as reference):
"trimPaste": true,(if you don't, the bot will still just merge this and I can take care of it in post
There was a problem hiding this comment.
No problm, I added "trimPaste": true,. For it to make sense I also changed the default value in code as per your suggested change
There was a problem hiding this comment.
Do I have to change this setting through the json file, as the doc suggests? I don't see a possibility do set it through the settings dialog.
I'm experiencing the behaviuor for a few days and I am surprised to see the changed behavour was introduced in October 21, already.
There was a problem hiding this comment.
| if (pos == textView.npos) | ||
| { | ||
| // Text is all white space, set empty string | ||
| text = L""; |
There was a problem hiding this comment.
we can probably just bail all the way out of pasting, right? Or do we need to complete the event handler request?
There was a problem hiding this comment.
Added co_return, seems to work as expected
zadjii-msft
added
the
AutoMerge
|
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 (
|
|
🎉 Handy links: |
|
Question, what about leading whitespace? Just this bit of powershell code as example: function Send-Bytes {
param(
[string] $To,
[string] $Arguments
)
begin {
$startInfo = [System.Diagnostics.ProcessStartInfo]::new()
$startInfo.RedirectStandardInput = $true
$startInfo.FileName = $To
$startInfo.Arguments = $Arguments
$process = [System.Diagnostics.Process]::Start($startInfo)
$inputStream = $process.StandardInput.BaseStream
}
process {
$inputStream.WriteByte([byte]$_)
}
end {
$inputStream.Flush()
$inputStream.Close()
$process.WaitForExit()
}
}It's indented with tabs, if I use Pasting the same thing in Terminal trims all indentation. |
6 participants
Summary of the Pull Request
Opt in setting to trim trailing white space when pasting a text into the terminal
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Manually testing to paste text with and without trailing white spaces, with and without the option activated