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.

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

Modernize TextBuffer's GenHTML() function #1846

Closed
carlos-zamora opened this issue Jul 5, 2019 · 1 comment
Closed

Modernize TextBuffer's GenHTML() function #1846

carlos-zamora opened this issue Jul 5, 2019 · 1 comment
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@carlos-zamora
Copy link
Member

Summary of the new feature/enhancement

There's a bit of refactoring that needs to get done for this GenHTML() function. I'm moving it to TextBuffer in PR #1224. So after that goes in, it'll be fair game.

Proposed technical implementation details (optional)

Some of the work suggested by fcharlie and mauve:

  • change some std::string const to constexpr std::wstring_view
  • move some variables to point of first use
  • remove Hungarian notation NL.5: Avoid encoding type information in names
  • resize, format, resize again seems cumbersome. Look into using C++ streams instead
@carlos-zamora carlos-zamora added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. labels Jul 5, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jul 5, 2019
@carlos-zamora carlos-zamora added Help Wanted We encourage anyone to jump in on these. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jul 5, 2019
@mcpiroman mcpiroman mentioned this issue Jul 19, 2019
Merged
5 tasks
@ghost ghost added the In-PR This issue has a related PR label Jul 19, 2019
DHowett-MSFT pushed a commit that referenced this issue Aug 6, 2019
@mcpiroman @DHowett
Refactor TextBuffer::GenHTML (#2038)
fa89e53 
Fixes #1846.
@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 Aug 6, 2019
@ghost ghost closed this as completed in 71eaf62 Aug 19, 2019
@ghost
Copy link

ghost commented Aug 27, 2019

🎉This issue was addressed in #2038, which has now been successfully released as Windows Terminal Preview v0.4.2382.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
Assignees
No one assigned
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Help Wanted We encourage anyone to jump in on these. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase 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
Milestone
No milestone
Development

No branches or pull requests
1 participant
@carlos-zamora