Skip to content

Task: Remove unused methods in ConGetSet and SCREEN_INFORMATION #9771

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.

Sign up for GitHub

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

Jump to bottom
Closed
j4james opened this issue Apr 10, 2021 · 1 comment · Fixed by #9772
Closed

Task: Remove unused methods in ConGetSet and SCREEN_INFORMATION #9771

j4james opened this issue Apr 10, 2021 · 1 comment · Fixed by #9772
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. 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 Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@j4james
Copy link
Collaborator

j4james commented Apr 10, 2021

Description of the new feature/enhancement

I've been investigating steps that could be taken to simplify the ConGetSet interface, with a view towards merging the AdaptDispatch and TerminalDispatch classes (#3849), and I noticed that the GetConsoleCursorInfo and SetConsoleCursorInfo methods are not actually used anywhere. While it's possible we may need something of similar functionality in the future, we wouldn't want it tied to a conhost-specific structure like CONSOLE_CURSOR_INFO if we're going to share the interface with TerminalDispatch, so I think it's probably best to just nuke them for now.

I also noticed that the SCREEN_INFORMATION::GetScrollingRegion is not used anywhere either. It was originally called from the ScrollRegion function, but that usage was removed in PR #2764, so I definitely think it's safe to delete this one.

Proposed technical implementation details (optional)

If there are no objections, I would like to remove these methods.
@j4james j4james added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Apr 10, 2021
@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 Apr 10, 2021
@j4james j4james mentioned this issue Apr 10, 2021
Merged
@ghost ghost added the In-PR This issue has a related PR label Apr 10, 2021
@DHowett
Copy link
Member

DHowett commented Apr 11, 2021

I love this.

@ghost ghost closed this as completed in #9772 Apr 12, 2021
@ghost ghost removed the In-PR This issue has a related PR label Apr 12, 2021
ghost pushed a commit that referenced this issue Apr 12, 2021
@j4james
Remove unused methods in ConGetSet and SCREEN_INFORMATION (#9772)
83bd241 
This PR removes the `GetConsoleCursorInfo` and `SetConsoleCursorInfo`
methods from the `ConGetSet` interface, and the `GetScrollingRegion`
method from the `SCREEN_INFORMATION` class. None of these methods are
used anymore.

PR #2764 removed the last usage of `GetScrollingRegion`. 

The `Get/SetConsoleCursorInfo` methods don't seem to have ever been used
in the time that the terminal code has been open source, so whatever
purpose they originally served must have been replaced a long time ago.

The `GetScrollingRegion` method was originally called from the
`ScrollRegion` function, but that usage was removed in PR #2764 when the
margin handling was moved to a higher level.

I've checked that the code still compiles.

Closes #9771
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Apr 12, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. 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 Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove unused methods in ConGetSet and SCREEN_INFORMATION j4james/terminal
2 participants
@DHowett @j4james