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

Allow "vintage" cursor to have values lower than 25 #9175

Closed
trajano opened this issue Feb 15, 2021 · 6 comments · Fixed by #9386
Closed

Allow "vintage" cursor to have values lower than 25 #9175

trajano opened this issue Feb 15, 2021 · 6 comments · Fixed by #9386
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) 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. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@trajano
Copy link

trajano commented Feb 15, 2021

Description of the new feature/enhancement

Allow "vintage" cursor to have values lower than 25, it is presently capped between 25 to 100. I just want to have the capability of setting it to a smaller value that is larger than underline

Proposed technical implementation details (optional)

Remove the bottom limit and allow floating point values.

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

DHowett commented Feb 15, 2021

The vintage cursor is intended to provide compatibility with folks' expectations of the windows console. It's not meant to be a good fun and expressive cursor for people to personalize with. What specifically are you looking for? Why do you want this capability?

and allow floating point values

So, you want a cursor with a fractional percentage height? I'm gonna say no to that one.

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 15, 2021
@trajano
Copy link
Author

trajano commented Feb 15, 2021

Well can you at least make it allow 1-24?

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Feb 15, 2021
@zadjii-msft
Copy link
Member

I guess I don't see why not

@zadjii-msft zadjii-msft added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) 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. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Feb 16, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 16, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Feb 16, 2021
@zadjii-msft zadjii-msft removed Needs-Attention The core contributors need to come back around and look at this ASAP. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Feb 16, 2021
@maggch97
Copy link

maggch97 commented Mar 3, 2021

vintage

Windows Terminal Preview v1.7.572.0

allowed input range may be too large

@ghost ghost added the In-PR This issue has a related PR label Mar 4, 2021
@ghost ghost closed this as completed in #9370 Mar 4, 2021
ghost pushed a commit that referenced this issue Mar 4, 2021
## Summary of the Pull Request
Add `Minimum` and `Maximum` for the cursor height numberbox in the SUI.
Add `Minimum` for the history size numberbox in the SUI.

## PR Checklist
* [x] Closes #9357, Closes #9175
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA

## Validation Steps Performed
Manual validation
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Mar 4, 2021
@zadjii-msft
Copy link
Member

Mmmm nope, this wasn't closed by #9370. We still need to update _drawCursor in CustomTextRenderer.cpp to allow MinCursorHeightPercent to be 1

@zadjii-msft zadjii-msft reopened this Mar 4, 2021
@zadjii-msft zadjii-msft removed the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Mar 4, 2021
@ghost ghost added the In-PR This issue has a related PR label Mar 5, 2021
@ghost ghost closed this as completed in #9386 Mar 9, 2021
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Mar 9, 2021
ghost pushed a commit that referenced this issue Mar 9, 2021
Change the vintage cursor height number box to a slider.

## References
Related:  #9370

## PR Checklist
* [x] Closes #9377
* [x] zadjii-msft edit: Now _this one_ closes #9175
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Schema updated.
* [ ] 

## Detailed Description of the Pull Request / Additional comments

It seems like the cursor height couldn't be lower than 25 percent regardless of the given value, so I've changed the `MinCursorHeightPercent` in CustomTextRenderer header file.

## Validation Steps Performed
Manual validation

![CursorHeightSlider](https://user-images.githubusercontent.com/39456018/110041939-bf076080-7d66-11eb-8d58-ba9a84922803.gif)
DHowett pushed a commit that referenced this issue Apr 13, 2021
## Summary of the Pull Request
Add `Minimum` and `Maximum` for the cursor height numberbox in the SUI.
Add `Minimum` for the history size numberbox in the SUI.

## PR Checklist
* [x] Closes #9357, Closes #9175
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA

## Validation Steps Performed
Manual validation

(cherry picked from commit ac3fecb)
@ghost
Copy link

ghost commented Apr 14, 2021

🎉This issue was addressed in #9386, which has now been successfully released as Windows Terminal Preview v1.8.1032.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) 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. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants