Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy RTF data to the clipboard #3535

Merged
merged 4 commits into from
Nov 13, 2019
Merged

Conversation

anirudhrb
Copy link
Contributor

@anirudhrb anirudhrb commented Nov 12, 2019

Summary of the Pull Request

RTF data is now copied to the clipboard. Tested by copy pasting text from terminal to WordPad.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Mostly similar to PR #1224. Added a new static method GenRTF in TextBuffer that is responsible
for generating the RTF representation of a given text. The generated RTF is added to the DataPackage that is ultimately passed to the clipboard.

Validation Steps Performed

Validated by copy pasting text from the terminal to WordPad. Validated with different colors to make sure that is working. (MS Word seems to prefer HTML data from the clipboard instead of RTF.)

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks! I feel like there's a way to share code between GenHTML() and GenRTF() but I don't think that's worth blocking over. I created #3538 to track that.

@DHowett-MSFT Thoughts on bringing this into ConHost too? I feel like that'd be a separate PR but definitely relevant.

src/buffer/out/textBuffer.cpp Show resolved Hide resolved
src/buffer/out/textBuffer.hpp Show resolved Hide resolved
@DHowett-MSFT
Copy link
Contributor

Yes, this will be cool for conhost.

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me. Thanks for doing this!

@zadjii-msft
Copy link
Member

@anirudhrb it looks like the code formatting check is failing - could you run tools\runformat in the root of the repo?

@zadjii-msft zadjii-msft added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Product-Terminal The new Windows Terminal. labels Nov 13, 2019
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 13, 2019
@anirudhrb
Copy link
Contributor Author

Yes, this will be cool for conhost.

@DHowett-MSFT @carlos-zamora What needs to be done to bring this into conhost? Are you gonna create a new issue?

@DHowett-MSFT
Copy link
Contributor

Followup filed at #3560. No need to do it in this one. 😄

@carlos-zamora
Copy link
Member

carlos-zamora commented Nov 13, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zadjii-msft zadjii-msft merged commit 13406b7 into microsoft:master Nov 13, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

🎉Windows Terminal Preview v0.7.3291.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Rich Text Format (RTF) Copy
4 participants