Add options to enable and disable read only mode #14995

Swinkid · 2023-03-14T23:40:56Z

Summary of the Pull Request

PR adds functionality to enable or disable readOnly mode within panes. This functionality is different to toggling as if you call the same functionality twice, it will not toggle between states.

References and Relevant Issues

Closes Add Enable/Disable actions alongside "Toggle pane read-only mode" #14415
Documentation Add documentation to include information about enable/disable read-only mode functionality MicrosoftDocs/terminal#645

Validation Steps Performed

Checked readOnly is enabled when command triggered
Checked readOnly is enabled when command triggered while read only already enabled
Checked readOnly is disabled when command triggered while read only is enabled
Checked readOnly stays disabled when command triggered while read only is disabled
Checked above with multiple tabs and split panes

PR Checklist

Closes Add Enable/Disable actions alongside "Toggle pane read-only mode" #14415
Tests added/passed
Documentation updated
- If checked, please file a pull request on our docs repo and link it here: Add documentation to include information about enable/disable read-only mode functionality MicrosoftDocs/terminal#645
Schema updated (if necessary)

Swinkid · 2023-03-14T23:41:35Z

@microsoft-github-policy-service agree

Swinkid · 2023-03-15T09:32:26Z

@zadjii-msft Not sure why PR is failing, unrelated to my code change?

src/cascadia/TerminalApp/TerminalTab.cpp

lhecker · 2023-03-16T16:51:09Z

Not sure why PR is failing, unrelated to my code change?

This will be fixed #15002. You can merge with main once that PR is merged.
Edit: I merged it by hand so you can use it right away.

…nged from PR.

Swinkid · 2023-03-16T17:59:33Z

@lhecker Made changes on the back of your review - Has been pushed and is building now.

Merged latest main into branch, but now getting an error to do with my code formatting - resolving that now.

Swinkid · 2023-03-20T13:14:07Z

@lhecker @zadjii-msft Whats the next steps? Is there something I need to do to get this progressed?

zadjii-msft

Honestly, just the one question. The rest looks great to me!

zadjii-msft · 2023-03-20T15:15:42Z

src/cascadia/TerminalApp/TerminalTab.cpp

+ void TerminalTab::SetPaneReadOnly(const bool readOnlyState)
+ {
+ auto hasReadOnly = false;
+ auto allReadOnly = true;
+ _activePane->WalkTree([&](const auto& p) {
+ if (const auto& control{ p->GetTerminalControl() })
+ {
+ hasReadOnly |= control.ReadOnly();
+ allReadOnly &= control.ReadOnly();
+ }
+ });
+ _activePane->WalkTree([&](const auto& p) {
+ if (const auto& control{ p->GetTerminalControl() })
+ {
+ // If all controls have the same read only state then just disable
+ if (allReadOnly || !hasReadOnly)
+ {
+ control.SetReadOnly(readOnlyState);
+ }
+ // otherwise set to all read only.
+ else if (!control.ReadOnly())
+ {
+ control.SetReadOnly(readOnlyState);
+ }
+ }
+ });
+ }
+


Is all of this logic necessary? Why wouldn't this just be a

WalkTree([&](const auto& p){ if (const auto& control{ p->GetTerminalControl() }) { control.SetReadOnly(readOnlyState); } });

Maybe I just haven't had enough coffee yet

Ill push my changes for this, I dont see any issue when ive tested it.

src/cascadia/TerminalControl/ControlCore.cpp

Co-authored-by: Mike Griese <migrie@microsoft.com>

…nkid/terminal into dev/swinkid/readonly-actions

This reverts commit 1670299.

This reverts commit 69787f0.

This reverts commit 8a8590a.

Swinkid · 2023-03-20T21:02:10Z

@zadjii-msft If i take your changes about the tree into account it fails the tests - they must be related to something but can't figure out what. I've tested the changes but it works as it should.

zadjii-msft · 2023-03-22T14:20:17Z

If i take your changes about the tree into account it fails the tests - they must be related to something but can't figure out what. I've tested the changes but it works as it should.

That's w a c k y but okay! Thanks for confirming

DHowett

Hey, thanks so much for doing this!

Swinkid · 2023-03-23T01:21:04Z

Hey, thanks so much for doing this!

You're welcome - this is why I love opensource.. helping fellow developers out!

## Summary of the Pull Request PR adds functionality to enable or disable readOnly mode within panes. This functionality is different to toggling as if you call the same functionality twice, it will not toggle between states. ## References and Relevant Issues - Closes #14415 - Documentation MicrosoftDocs/terminal#645 ## Validation Steps Performed - Checked readOnly is enabled when command triggered - Checked readOnly is enabled when command triggered while read only already enabled - Checked readOnly is disabled when command triggered while read only is enabled - Checked readOnly stays disabled when command triggered while read only is disabled - Checked above with multiple tabs and split panes ## PR Checklist - [ ] Closes #14415 - [X] Tests added/passed - [x] Documentation updated - If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: MicrosoftDocs/terminal#645 - [X] Schema updated (if necessary) --------- Co-authored-by: Mike Griese <migrie@microsoft.com> (cherry picked from commit 2acdc9d) Service-Card-Id: 89002012 Service-Version: 1.17

Add options to enable and disable read only mode

f0a2419

Swinkid mentioned this pull request Mar 14, 2023

Add Enable/Disable actions alongside "Toggle pane read-only mode" #14415

Closed

Swinkid added 2 commits March 15, 2023 00:19

Add missing handlers and shortcut actions

e9fcd4e

Add missing elements and add missing comments

f1e9d94

Swinkid marked this pull request as ready for review March 15, 2023 00:57

Merge branch 'main' into dev/swinkid/readonly-actions

fa5e51b

microsoft-github-policy-service bot added Issue-Task It's a feature request, but it doesn't really need a major design. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Mar 15, 2023

Swinkid mentioned this pull request Mar 15, 2023

Add documentation to include information about enable/disable read-only mode functionality MicrosoftDocs/terminal#645

Merged

lhecker reviewed Mar 16, 2023

View reviewed changes

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

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

Swinkid added 2 commits March 16, 2023 16:54

Merge branch 'main' into dev/swinkid/readonly-actions

2d7ceca

Refactored Enable/Disable functions to Set function. Incorporated cha…

2035e7c

…nged from PR.

Running Invoke-CodeFormat

7fa1579

lhecker approved these changes Mar 16, 2023

View reviewed changes

zadjii-msft reviewed Mar 20, 2023

View reviewed changes

Swinkid and others added 7 commits March 20, 2023 17:07

Update src/cascadia/TerminalControl/ControlCore.cpp

2990281

Co-authored-by: Mike Griese <migrie@microsoft.com>

Update TerminalTab.cpp

1670299

Merge branch 'dev/swinkid/readonly-actions' of https://github.com/Swi…

13445d7

…nkid/terminal into dev/swinkid/readonly-actions

Revert "Update TerminalTab.cpp"

69787f0

This reverts commit 1670299.

Merge branch 'main' into dev/swinkid/readonly-actions

2903026

Revert "Revert "Update TerminalTab.cpp""

8a8590a

This reverts commit 69787f0.

Revert "Revert "Revert "Update TerminalTab.cpp"""

b17b79f

This reverts commit 8a8590a.

zadjii-msft approved these changes Mar 22, 2023

View reviewed changes

zadjii-msft merged commit 2acdc9d into microsoft:main Mar 22, 2023

DHowett reviewed Mar 23, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add options to enable and disable read only mode #14995

Add options to enable and disable read only mode #14995

Swinkid commented Mar 14, 2023 •

edited

Loading

Swinkid commented Mar 14, 2023

Swinkid commented Mar 15, 2023

lhecker commented Mar 16, 2023 •

edited

Loading

Swinkid commented Mar 16, 2023 •

edited

Loading

Swinkid commented Mar 20, 2023

zadjii-msft left a comment

zadjii-msft Mar 20, 2023

Swinkid Mar 20, 2023

Swinkid commented Mar 20, 2023 •

edited

Loading

zadjii-msft commented Mar 22, 2023

DHowett left a comment

Swinkid commented Mar 23, 2023

Add options to enable and disable read only mode #14995

Add options to enable and disable read only mode #14995

Conversation

Swinkid commented Mar 14, 2023 • edited Loading

Summary of the Pull Request

References and Relevant Issues

Validation Steps Performed

PR Checklist

Swinkid commented Mar 14, 2023

Swinkid commented Mar 15, 2023

lhecker commented Mar 16, 2023 • edited Loading

Swinkid commented Mar 16, 2023 • edited Loading

Swinkid commented Mar 20, 2023

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Mar 20, 2023

Choose a reason for hiding this comment

Swinkid Mar 20, 2023

Choose a reason for hiding this comment

Swinkid commented Mar 20, 2023 • edited Loading

zadjii-msft commented Mar 22, 2023

DHowett left a comment

Choose a reason for hiding this comment

Swinkid commented Mar 23, 2023

Swinkid commented Mar 14, 2023 •

edited

Loading

lhecker commented Mar 16, 2023 •

edited

Loading

Swinkid commented Mar 16, 2023 •

edited

Loading

Swinkid commented Mar 20, 2023 •

edited

Loading