[PR #1224] [MERGED] Add support for HTML copy #24540

Open
opened 2026-01-31 09:03:54 +00:00 by claunia · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/microsoft/terminal/pull/1224
Author: @carlos-zamora
Created: 6/12/2019
Status: Merged
Merged: 8/19/2019
Merged by: @undefined

Base: masterHead: dev/cazamor/html-copy


📝 Commits (9)

📊 Changes

11 files changed (+247 additions, -271 deletions)

View changed files

📝 src/buffer/out/textBuffer.cpp (+177 -0)
📝 src/buffer/out/textBuffer.hpp (+5 -0)
📝 src/cascadia/TerminalApp/App.cpp (+13 -5)
📝 src/cascadia/TerminalApp/App.h (+1 -1)
📝 src/cascadia/TerminalControl/TermControl.cpp (+15 -4)
📝 src/cascadia/TerminalControl/TermControl.h (+19 -2)
📝 src/cascadia/TerminalControl/TermControl.idl (+7 -2)
📝 src/cascadia/TerminalCore/Terminal.hpp (+1 -1)
📝 src/cascadia/TerminalCore/TerminalSelection.cpp (+6 -14)
📝 src/interactivity/win32/Clipboard.cpp (+3 -241)
📝 src/interactivity/win32/clipboard.hpp (+0 -1)

📄 Description

Summary of the Pull Request

HTML data is now copied to the clipboard. I tried reusing the old clipboard method so I had to move some stuff around. The design is pretty straightforward. Instead of sending just text data up on a CopyToClipboard event, send the HTML data up too in the same winrt object.

Here's some specifics:

  • Clipboard::GenHTML() Changes:
    • moved from Clipboard to TextBuffer
    • added iFontHeightPoints and fontFaceName so that I don't depend on the ServiceLocator
  • TermControl: have CopyToClipboardEventArgs have space for both data types
  • TerminalApp: just extract the data from the event and put it into the clipboard

References

#1146 (HTML portion)

PR Checklist

  • Closes half of install problem in version1809 (#1146)
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Requires documentation to be 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.

Detailed Description of the Pull Request / Additional comments

I do have a few things I'm not sure of:

  • As of now, HTML copy is always enabled. I can easily add make it optional but idk what the design should look like (i.e.: new keybinding copies only HTML?)
  • I don't think we can/should add tests to this because (1) it resuses old code and (2) we definitely don't want to actually copy stuff to the clipboard during a test. The only thing to test would be what GenHTML does but that was already here.
  • Not sure if the iFontHeightPointsis right (or even described properly for that matter) in GenHTML().
    • for Clipboard we do int const iFontHeightPoints = fontData.GetUnscaledSize().Y * 72 / ServiceLocator::LocateGlobals().dpi;
    • for TermControl I did fontData.GetUnscaledSize().Y
    • I don't really know how to verify if this is correct. But I also don't know if we actually even need the DPI here. Thoughts?

Validation Steps Performed

Copy selections and paste to MSWord (regular text vs formatted text)


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/microsoft/terminal/pull/1224 **Author:** [@carlos-zamora](https://github.com/carlos-zamora) **Created:** 6/12/2019 **Status:** ✅ Merged **Merged:** 8/19/2019 **Merged by:** [@undefined](undefined) **Base:** `master` ← **Head:** `dev/cazamor/html-copy` --- ### 📝 Commits (9) - [`9116bdf`](https://github.com/microsoft/terminal/commit/9116bdf8a31adf21a9bf3bda277f8d96e81cb42f) Move Clipboard::GenHTML to TextBuffer (add params) - [`44e3da8`](https://github.com/microsoft/terminal/commit/44e3da83fc40843be321f8e6e3a9800047e50908) minor code format fix - [`514d007`](https://github.com/microsoft/terminal/commit/514d007ae8779035d5a99de8c227449c6755045b) PR Changes - [`fa89e53`](https://github.com/microsoft/terminal/commit/fa89e53fa665515728548813f0d76a6639d16846) Refactor TextBuffer::GenHTML (#2038) - [`a95d1cb`](https://github.com/microsoft/terminal/commit/a95d1cb2f49be3f8de06394df4ed6a6446cf8ef4) Merge branch 'master' into dev/cazamor/html-copy - [`752be0f`](https://github.com/microsoft/terminal/commit/752be0f15dc3a0aa6e7924d9698d84f98b0d789a) nit change - [`cdda59c`](https://github.com/microsoft/terminal/commit/cdda59c025e485353feca15cb6110550b1093fc9) x86 build fix - [`7f7fcfc`](https://github.com/microsoft/terminal/commit/7f7fcfcc9fb8afb614612845b840cf7b161975b1) Merge branch 'master' into dev/cazamor/html-copy - [`e93cb5c`](https://github.com/microsoft/terminal/commit/e93cb5c3502afa7089bc15c7b032f0e5d871030e) nit changes ### 📊 Changes **11 files changed** (+247 additions, -271 deletions) <details> <summary>View changed files</summary> 📝 `src/buffer/out/textBuffer.cpp` (+177 -0) 📝 `src/buffer/out/textBuffer.hpp` (+5 -0) 📝 `src/cascadia/TerminalApp/App.cpp` (+13 -5) 📝 `src/cascadia/TerminalApp/App.h` (+1 -1) 📝 `src/cascadia/TerminalControl/TermControl.cpp` (+15 -4) 📝 `src/cascadia/TerminalControl/TermControl.h` (+19 -2) 📝 `src/cascadia/TerminalControl/TermControl.idl` (+7 -2) 📝 `src/cascadia/TerminalCore/Terminal.hpp` (+1 -1) 📝 `src/cascadia/TerminalCore/TerminalSelection.cpp` (+6 -14) 📝 `src/interactivity/win32/Clipboard.cpp` (+3 -241) 📝 `src/interactivity/win32/clipboard.hpp` (+0 -1) </details> ### 📄 Description <!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request HTML data is now copied to the clipboard. I tried reusing the old clipboard method so I had to move some stuff around. The design is pretty straightforward. Instead of sending just text data up on a `CopyToClipboard` event, send the HTML data up too in the same winrt object. Here's some specifics: - `Clipboard::GenHTML()` Changes: - moved from `Clipboard` to `TextBuffer` - added `iFontHeightPoints` and `fontFaceName` so that I don't depend on the `ServiceLocator` - `TermControl`: have `CopyToClipboardEventArgs` have space for both data types - `TerminalApp`: just extract the data from the event and put it into the clipboard <!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> ## References #1146 (HTML portion) <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist * [x] Closes half of #1146 * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Requires documentation to be updated * [x] 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. <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments I do have a few things I'm not sure of: - As of now, HTML copy is always enabled. I can easily add make it optional but idk what the design should look like (i.e.: new keybinding copies only HTML?) - I don't think we can/should add tests to this because (1) it resuses old code and (2) we definitely don't want to actually copy stuff to the clipboard during a test. The only thing to test would be what `GenHTML` does but that was already here. - Not sure if the `iFontHeightPoints`is right (or even described properly for that matter) in `GenHTML()`. - for `Clipboard` we do `int const iFontHeightPoints = fontData.GetUnscaledSize().Y * 72 / ServiceLocator::LocateGlobals().dpi;` - for `TermControl` I did `fontData.GetUnscaledSize().Y` - I don't really know how to verify if this is correct. But I also don't know if we actually even need the DPI here. Thoughts? ## Validation Steps Performed Copy selections and paste to MSWord (regular text vs formatted text) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
claunia added the pull-request label 2026-01-31 09:03:54 +00:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#24540