Trim trailing whitespace option #11473

lovef · 2021-10-10T13:44:14Z

Summary of the Pull Request

Opt in setting to trim trailing white space when pasting a text into the terminal

References

PR Checklist

Closes Add a third option which replaces newlines by spaces when pasting content with multiple lines #9400
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
Schema updated.
I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

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

src/cascadia/TerminalApp/TerminalPage.cpp

zadjii-msft · 2021-10-26T21:09:41Z

@msftbot don't merge this before friday at 7pm

ghost · 2021-10-26T21:09:43Z

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.

zadjii-msft · 2021-10-26T21:10:03Z

@msftbot merge this friday at 7pm

ghost · 2021-10-26T21:10:05Z

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

Thanks for whipping this up!

zadjii-msft · 2021-10-26T21:11:41Z

src/cascadia/TerminalSettingsModel/GlobalAppSettings.h

@@ -77,6 +77,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
        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);


Suggested change

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 ☺️)

No problm, I added "trimPaste": true,. For it to make sense I also changed the default value in code as per your suggested change

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.

@bardware Which version are you using? The option should be right here:

On the "Interaction" page

DHowett · 2021-10-26T21:15:21Z

src/cascadia/TerminalApp/TerminalPage.cpp

+                if (pos == textView.npos)
+                {
+                    // Text is all white space, set empty string
+                    text = L"";


we can probably just bail all the way out of pasting, right? Or do we need to complete the event handler request?

With a co_return?

Added co_return, seems to work as expected

ghost · 2021-10-28T15:37:16Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (`@msftbot`) and give me an instruction to get started! Learn more here.

ghost · 2022-02-03T18:27:27Z

🎉Windows Terminal Preview v1.13.10336.0 has been released which incorporates this pull request.:tada:

Handy links:

Hrxn · 2023-09-26T23:49:11Z

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 pwsh.exe (i.e. the old ConHost) and do a multiline paste, the indentation is preserved - almost: the leading tabs are converted to spaces.

Pasting the same thing in Terminal trims all indentation.
I mean, it's not a functional change, but it looks a bit weird, to be honest.

Windows Terminal
Version: 1.17.11461.0

ghost added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Oct 10, 2021

lhecker reviewed Oct 12, 2021

View reviewed changes

src/cascadia/TerminalApp/TerminalPage.cpp Outdated Show resolved Hide resolved

lovef force-pushed the trim-paste branch 2 times, most recently from 18bea3b to 6454f73 Compare October 12, 2021 16:23

lhecker approved these changes Oct 12, 2021

View reviewed changes

lhecker added the Needs-Second It's a PR that needs another sign-off label Oct 20, 2021

ghost requested review from zadjii-msft, miniksa, carlos-zamora, DHowett, leonMSFT and PankajBhojwani October 20, 2021 20:10

zadjii-msft approved these changes Oct 26, 2021

View reviewed changes

DHowett reviewed Oct 26, 2021

View reviewed changes

lovef force-pushed the trim-paste branch from 6454f73 to b4ac90d Compare October 26, 2021 21:40

Trim trailing whitespace option

1094181

lovef force-pushed the trim-paste branch from b4ac90d to 1094181 Compare October 26, 2021 21:51

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 28, 2021

ghost merged commit 756fd44 into microsoft:main Oct 28, 2021

carlos-zamora mentioned this pull request Oct 28, 2021

Add trimPaste to docs and schema #11642

Closed

zadjii-msft mentioned this pull request Oct 28, 2021

add trimPaste to the docs MicrosoftDocs/terminal#435

Merged

zadjii-msft mentioned this pull request Dec 9, 2021

An option to swallow the trailing CRLF on paste #5120

Closed

zadjii-msft mentioned this pull request Jan 18, 2022

Option to paste without the final newline #12185

Closed

ghost mentioned this pull request Feb 3, 2022

Add a third option which replaces newlines by spaces when pasting content with multiple lines #9400

Closed

fischerdouglas mentioned this pull request Feb 4, 2022

Paste Slowly - A new type of Action #11020

Open

This was referenced Feb 8, 2022

Pasting multiple lines doesn't execute the last line #12387

Closed

Cannot get trimPaste to work (I am running preview) #12467

Closed

ghost removed the Needs-Second It's a PR that needs another sign-off label Jun 19, 2022

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trim trailing whitespace option #11473

Trim trailing whitespace option #11473

lovef commented Oct 10, 2021

zadjii-msft commented Oct 26, 2021

ghost commented Oct 26, 2021

zadjii-msft commented Oct 26, 2021

ghost commented Oct 26, 2021

zadjii-msft left a comment

zadjii-msft Oct 26, 2021

lovef Oct 26, 2021

bardware Jun 19, 2022

zadjii-msft Jun 20, 2022

DHowett Oct 26, 2021

lovef Oct 26, 2021

lovef Oct 26, 2021

ghost commented Oct 28, 2021

ghost commented Feb 3, 2022

Hrxn commented Sep 26, 2023

	INHERITABLE_SETTING(Model::GlobalAppSettings, bool, TrimPaste, false);
	INHERITABLE_SETTING(Model::GlobalAppSettings, bool, TrimPaste, true);

Trim trailing whitespace option #11473

Trim trailing whitespace option #11473

Conversation

lovef commented Oct 10, 2021

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

zadjii-msft commented Oct 26, 2021

ghost commented Oct 26, 2021

zadjii-msft commented Oct 26, 2021

ghost commented Oct 26, 2021

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Oct 26, 2021

Choose a reason for hiding this comment

lovef Oct 26, 2021

Choose a reason for hiding this comment

bardware Jun 19, 2022

Choose a reason for hiding this comment

zadjii-msft Jun 20, 2022

Choose a reason for hiding this comment

DHowett Oct 26, 2021

Choose a reason for hiding this comment

lovef Oct 26, 2021

Choose a reason for hiding this comment

lovef Oct 26, 2021

Choose a reason for hiding this comment

ghost commented Oct 28, 2021

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 (@msftbot) and give me an instruction to get started! Learn more here.

ghost commented Feb 3, 2022

Hrxn commented Sep 26, 2023

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 (`@msftbot`) and give me an instruction to get started! Learn more here.