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

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

Closed
ev-dev opened this issue Nov 20, 2022 · 5 comments · Fixed by #14995
Closed

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

ev-dev opened this issue Nov 20, 2022 · 5 comments · Fixed by #14995
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Milestone

Comments

@ev-dev
Copy link

ev-dev commented Nov 20, 2022

Description of the new feature/enhancement

Two additional terminal actions:

  • Enable pane read-only mode
  • Disable pane read-only mode

Reasoning

I make use of the Read-Only mode occasionally, usually whenever I have multiple terminal windows open and a long-running command running in one which prevents me from accidentally hitting "Ctrl+C" in the unintended window.

Read-Only mode has the nice feature of showing a padlock icon next to the pane's title whenever Read-Only mode is active, but of course this helpful hint is not visible if the terminal is in Full-Screen mode.

Whenever that scenario (toggling read-only mode in a fullscreen window) comes up for me, I'll usually just hit space, which will show a pop-up if the pane is in Read-Only mode, and if no pop-up is shown, I'll know to run the action to toggle read-only mode.

I think it would be helpful to have the 2 actions I proposed above, neither of which should care whether Read-Only mode is already active or not, and then internally either perform the toggle of read-only mode, or noop. These actions would provide a more assured method of setting the intended mode for a pane and don't need to prompt the user.

Proposed technical implementation details (optional)

My current workaround is a custom terminal action (defined in my settings.json):

"actions": [
    {
        "name": "Toggle pane read-only mode (w/check)",
        "keys": "alt+r",
        "command": {
            "action": "multipleActions",
            "actions": [
                { "action": "toggleReadOnlyMode" },
                { "action": "sendInput", "input": " " }
            ]
        }
    },
]

This gives action provides an indication of the Read-Only mode state of a pane by forcefully showing the "Read-only mode is enabled" pop-up after sending a space, and if nothing happens when I run the action, I know I need to run it again.

I couldn't find any mechanism of setting/retrieving the current state of a pane's read-only mode, so I wasn't able to figure out an alternative solution for myself (for instance, defining a PowerShell function to check/toggle the read-only mode state, which could maybe be used in a Terminal action)

Given that the "Toggle pane read-only mode" is a pretty stable feature, it may be pretty simple to add these additional actions within the source code here. I'll give it my best shot and report back but I wanted to see if anyone had an opinion/suggestion as well.

Thanks to all the contributors for all the great work you do on this project!

@ev-dev ev-dev added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Nov 20, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Nov 20, 2022
@zadjii-msft
Copy link
Member

This is such a reasonable and well written proposal, I can't help but say "sure, why not". The code wouldn't be hard to write (just a lot of plumbing) nor maintain.

@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. good first issue This is a fix that might be easier for someone to do as a first contribution and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 28, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 28, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Nov 28, 2022
@Swinkid
Copy link
Contributor

Swinkid commented Mar 14, 2023

@zadjii-msft I think I am confident to be able to contribute to this. Would be my first contribution so bare with while I figure things out!

@zadjii-msft
Copy link
Member

zadjii-msft commented Mar 14, 2023

Absolutely, go for it! Lemme know if you need any pointers.

Note

Walkthrough

  • Add an action for immediately restarting a connection #14549 is a great example of where to start - that's another PR that adds an action and plumbs it through the settings model and hooks up the implementation from TerminalPage -> TermControl
  • Except, instead of "RestartConnection", it'd be like two actions EnableReadOnlyMode and DisableReadOnlyMode.

@Swinkid
Copy link
Contributor

Swinkid commented Mar 15, 2023

Changes made in #14995

Working locally - just need to update docs and what not

@Swinkid
Copy link
Contributor

Swinkid commented Mar 15, 2023

Added documentation in MicrosoftDocs/terminal#645

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Mar 15, 2023
zadjii-msft added a commit that referenced this issue Mar 22, 2023
## 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>
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Mar 22, 2023
DHowett pushed a commit that referenced this issue Apr 25, 2023
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants