Allow exporting terminal buffer into file via tab context menu #11062

Don-Vito · 2021-08-27T06:45:49Z

Summary of the Pull Request

Naive implementation of exporting the text buffer of the current pane
into a text file triggered from the tab context menu.

Disclaimer: this is not an export of the command history,
but rather just a text buffer dumped into a file when asked explicitly.

References

Should provide partial solution for #642.

Detailed Description of the Pull Request / Additional comments

The logic is following:

Open a file save picker
- The location is Downloads folder (should be always accessible)
- The suggest name of the file equals to the pane's title
- The allowed file formats list contains .txt only
If no file selected stop
Lock terminal
Read all lines till the cursor
Format each line by removing trailing white-spaces and adding CRLF if not wrapped
Asynchronously write to selected file
Show confirmation

As the action is relatively fast didn't add a progress bar or any other UX.
As the buffer is relatively small, holding it entirely in the memory rather than
writing line by line to disk.

Closes #642

Don-Vito · 2021-08-27T06:58:41Z

@zadjii-msft, @DHowett

The implementation is quite naive and limited:
- It contains only the lines of the buffer
- It is not history: there are no timestamps or lines that were altered.
- It is not ongoing logging, you actually need to export the file manually
I am guessing there might be less people happy with this feature than the ones annoyed with its limitations
Yet probably this could be a good place to start with
Unrelated to the above
- I was not sure what is the most efficient way to export the buffer, so I took the simplest approach. Please let me know how it can be done better.
- I didn't bother to write file line by line as the buffer is small. Please let me know if you think this should be changes at this stage.

Don-Vito · 2021-08-27T07:03:44Z

The icon is:

Don-Vito · 2021-08-27T07:28:53Z

Actually I don't like it.. Probably need to move the export into Control itself

carlos-zamora

So excited for this feature to land! Thank you!

I was debating mentioning TextBuffer::GetText() but that seems too overly complicated than just iterating through the buffer and removing the whitespace directly. Not worth it.

src/cascadia/TerminalControl/ControlCore.h

DHowett

This one I think needs more discussion before merging. I know you called this out in the related issue/in a comment on this PR, but yeah --- TermControl right now has no true external hookups except for throwing text into a connection and getting text from that connection back on screen. Any communication with the outside world is mediated through its event handlers ("I want to paste!" "Copy this text to the clipboard for me") and interfaces (ITerminalConnection's read and write). In the long-term future, it would be ideal if we could make it even less coupled to Terminal-the-app; imagine somebody like Visual Studio taking a dependency on the UWP version of the control instead of WPF, and it having a file-writing-sized hole in it.

ControlCore will eventually run on Windows 7, because ~ ~ hand-wavey annoying things regarding the WPF control and how i want to unify all the interaction models ~ ~, as will ControlInteractivity ... which makes taking a hard dependency on the ~~pretty bad, if i am honest~~ W.S.Storage APIs harder for me to digest. I dunno.

Thoughts?

Don-Vito · 2021-08-27T20:35:58Z

@DHowett - I see your point. Before my last commit the logic was in the App. I am reverting it.

zadjii-msft · 2021-08-30T15:22:36Z

The implementation is quite naive and limited:

It contains only the lines of the buffer

It is not history: there are no timestamps or lines that were altered.

It is not ongoing logging, you actually need to export the file manually

I am guessing there might be less people happy with this feature than the ones annoyed with its limitations

I think that's absolutely fine. I think that incremental progress is more important than wholly complete in the first go. IMO this is a good place to start, while we think on how we want to actually express settings for the various options people all want. Auto-logging to files with formatted names, timestamps - that all basically needs a whole spec dedicated to it, but this can be done without all that planning.

@Don-Vito If you're interested in doing more auto-logging stuff, I can try and throw a spec together. Something more lightweight like #9365, so we can get you unblocked and a list of things to do. If you want to work on other stuff though, that's absolutely fine with me ☺️

zadjii-msft

Code-wise, yea this is perfect for what it should do. But now you've got me thinking of all the other related things that this should do and I'm excited for those. I might end up writing the spec anyways 😝

zadjii-msft · 2021-08-30T15:28:36Z

src/cascadia/TerminalApp/TabManagement.cpp

+            if (const auto control{ tab.GetActiveTerminalControl() })
+            {
+                const FileSavePicker savePicker;
+                savePicker.as<IInitializeWithWindow>()->Initialize(*_hostingHwnd);


Okay so just to be sure, this will work when the Terminal is elevated (run as admin), right? I want to make sure we don't run into the same issue that forced #9760

I do believe that it will crash.

It is pickers, of all sorts, through the W.S API that is broken, not any specific individual use. We may try/catch this one, but... the best we can do is simply not pop up a dialog when Admin.

It is pickers, of all sorts, through the W.S API that is broken, not any specific individual use. We may try/catch this one, but... the best we can do is simply not pop up a dialog when Admin.

Frick. I'll stick that on #9700

zadjii-msft · 2021-08-30T15:31:19Z

src/cascadia/TerminalApp/TabManagement.cpp

+                savePicker.as<IInitializeWithWindow>()->Initialize(*_hostingHwnd);
+                savePicker.SuggestedStartLocation(PickerLocationId::Downloads);
+                const auto fileChoices = single_threaded_vector<hstring>({ L".txt" });
+                savePicker.FileTypeChoices().Insert(RS_(L"PlainText"), fileChoices);


Wait should "Plain Text" be localizable? I guess it should be, huh.

zadjii-msft · 2021-08-30T15:31:45Z

src/cascadia/TerminalApp/TabManagement.cpp

+
+            if (page && tab)
+            {
+                page->_ExportTab(*tab);


an aside: we may want to (in a future PR) turn this into an action, that could let the user export the pane to a file from the command palette too, not just the context menu. But now I'm thinking of all the other kinds of actions we could have for this feature...

Don-Vito · 2021-08-30T17:07:52Z

Code-wise, yea this is perfect for what it should do. But now you've got me thinking of all the other related things that this should do and I'm excited for those. I might end up writing the spec anyways 😝

Cool! We absolutely need a spec to write! I would absolutely love to work on this feature (I need it as well). My major challenge is as usual: the time - I hope I will get to it (unlike the broadcast to multiple panes).

DIsmissing my own review in case folks want to move ahead like this :)

ghost · 2021-08-31T19:36:33Z

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.

DHowett · 2021-08-31T19:41:34Z

src/cascadia/TerminalApp/TerminalTab.cpp

@@ -1211,13 +1211,31 @@ namespace winrt::TerminalApp::implementation
            splitTabMenuItem.Icon(splitTabSymbol);
        }

+        Controls::MenuFlyoutItem exportTabMenuItem;
+        {
+            // "Split Tab"


nit: says "split" 😁

Just like in #9760, we can't actually use the UWP file picker API, because it will absolutely not work at all when the Terminal is running elevated. That would prevent the picker from appearing at all. So instead, we'll just use the shell32 one manually. This also gets rid of the confirmation dialog, since the team felt we didn't really need that. We could maybe replace it with a Toast (#8592), but _meh_ * [x] closes #11356 * [x] closes #11358 * This is a lot like #9760 * introduced in #11062 * megathread: #9700

ghost · 2021-10-20T17:06:55Z

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

Handy links:

This adds an action for the context menu entry we added in #11062. That PR added support for exporting the buffer, exclusively through the tab item's context menu. This adds an action that can additionally be bound, which also can export the buffer to a file. This action accepts a `path` param. If empty/ommitted, then the Terminal will prompt for the file to export the buffer to. * Does a part of #9700 * Spec in #11090, but I doubt this is contentious * [x] This will satisfy #12052 * [x] I work here * [x] docs added: MicrosoftDocs/terminal#479

### ⇒ [doc link](https://github.com/microsoft/terminal/blob/dev/migrie%2Fs%2F642-logging/doc/specs/drafts/%23642%20-%20Buffer%20Exporting%20and%20Logging/%23642%20-%20Buffer%20Exporting%20and%20Logging.md) ⇐ ## Summary of the Pull Request This is an intentionally brief spec to address the full scope of #642. The intention of this spec is to quickly build consensus around all the features we want for logging, and prepare an implementation plan. ### Abstract > A common user need is the ability to export the history of a terminal session to > a file, for later inspection or validation. This is something that could be > triggered manually. Many terminal emulators provide the ability to automatically > log the output of a session to a file, so the history is always captured. This > spec will address improvements to the Windows Terminal to enable these kinds of > exporting and logging scenarios. ## PR Checklist * [x] Specs: #642 * [x] References: #5000, #9287, #11045, #11062 * [x] I work here ## Detailed Description of the Pull Request / Additional comments _\*\*\* read the spec \*\*\*_ ## Open Discussion * [ ] What formatting string syntax and variables do we want to use? * [ ] How exactly do we want to handle "log printable output"? Do we include backspaces? Do we only log on newlines? * [ ] > maybe consider even simpler options like just `${date}` and `${time}`, and allow for future variants with something like `${date:yyyy-mm-dd}` or `${time:hhmm}`

Don-Vito added 2 commits August 27, 2021 09:26

Allow exporting terminal buffer into file via tab context menu

6351807

Postpone reading the text buffer post file selection

136ca87

Don-Vito marked this pull request as draft August 27, 2021 07:29

Move file storing into CoreControl

b64b840

Don-Vito marked this pull request as ready for review August 27, 2021 08:59

carlos-zamora approved these changes Aug 27, 2021

View reviewed changes

src/cascadia/TerminalControl/ControlCore.h Outdated Show resolved Hide resolved

DHowett previously requested changes Aug 27, 2021

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 27, 2021

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Aug 27, 2021

Revert moving the file operations to TermControl

f49c07b

Don-Vito requested review from DHowett and carlos-zamora August 27, 2021 20:39

zadjii-msft approved these changes Aug 30, 2021

View reviewed changes

zadjii-msft added Needs-Discussion Something that requires a team discussion before we can proceed and removed Needs-Discussion Something that requires a team discussion before we can proceed labels Aug 30, 2021

This was referenced Aug 31, 2021

megathread: Buffer exporting and logging #9700

Open

DRAFT Spec for Buffer Exporting and Logging #11090

Merged

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

ghost merged commit c089ae0 into microsoft:main Aug 31, 2021

DHowett reviewed Aug 31, 2021

View reviewed changes

zadjii-msft mentioned this pull request Sep 6, 2021

Feature request: export console history as txt #642

Closed

ghost added the Area-User Interface Issues pertaining to the user interface of the Console or Terminal label Sep 7, 2021

ghost added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Sep 7, 2021

This was referenced Sep 29, 2021

[1.12] Exporting text fails when running elevated (presumably we can't open the file picker elevated) #11356

Closed

Replace the UWP file export with the shell32 one #11365

Merged

zadjii-msft mentioned this pull request Jan 5, 2022

Add an exportBuffer action #12097

Merged

3 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow exporting terminal buffer into file via tab context menu #11062

Allow exporting terminal buffer into file via tab context menu #11062

Don-Vito commented Aug 27, 2021 •

edited by zadjii-msft

Loading

Don-Vito commented Aug 27, 2021 •

edited

Loading

Don-Vito commented Aug 27, 2021

Don-Vito commented Aug 27, 2021

carlos-zamora left a comment

DHowett left a comment •

edited

Loading

Don-Vito commented Aug 27, 2021

zadjii-msft commented Aug 30, 2021

zadjii-msft left a comment

zadjii-msft Aug 30, 2021

DHowett Aug 31, 2021

DHowett Aug 31, 2021

zadjii-msft Aug 31, 2021

zadjii-msft Aug 30, 2021

zadjii-msft Aug 30, 2021

Don-Vito commented Aug 30, 2021

ghost commented Aug 31, 2021

DHowett Aug 31, 2021

ghost commented Oct 20, 2021

Allow exporting terminal buffer into file via tab context menu #11062

Allow exporting terminal buffer into file via tab context menu #11062

Conversation

Don-Vito commented Aug 27, 2021 • edited by zadjii-msft Loading

Summary of the Pull Request

References

Detailed Description of the Pull Request / Additional comments

Don-Vito commented Aug 27, 2021 • edited Loading

Don-Vito commented Aug 27, 2021

Don-Vito commented Aug 27, 2021

carlos-zamora left a comment

Choose a reason for hiding this comment

DHowett left a comment • edited Loading

Choose a reason for hiding this comment

Don-Vito commented Aug 27, 2021

zadjii-msft commented Aug 30, 2021

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Aug 30, 2021

Choose a reason for hiding this comment

DHowett Aug 31, 2021

Choose a reason for hiding this comment

DHowett Aug 31, 2021

Choose a reason for hiding this comment

zadjii-msft Aug 31, 2021

Choose a reason for hiding this comment

zadjii-msft Aug 30, 2021

Choose a reason for hiding this comment

zadjii-msft Aug 30, 2021

Choose a reason for hiding this comment

Don-Vito commented Aug 30, 2021

ghost commented Aug 31, 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.

DHowett Aug 31, 2021

Choose a reason for hiding this comment

ghost commented Oct 20, 2021

Don-Vito commented Aug 27, 2021 •

edited by zadjii-msft

Loading

Don-Vito commented Aug 27, 2021 •

edited

Loading

DHowett left a comment •

edited

Loading

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.