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 an additional debug option to show a \n when \r is received #12312

Closed
eabase opened this issue Feb 1, 2022 · 7 comments · Fixed by #13475
Closed

Add an additional debug option to show a \n when \r is received #12312

eabase opened this issue Feb 1, 2022 · 7 comments · Fixed by #13475
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface 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.
Milestone

Comments

@eabase
Copy link

eabase commented Feb 1, 2022

Description of the new feature/enhancement

The debug tool is great, but could certainly be improved if you could add an option to actually do a \n when the \r (CR) character is received. This would significantly increase the readability of the infinite debug strings coming out, without affecting the actual output and understanding.

So that it would look like:

blah blahblah   blah
[C/R] blah blah

(Where [C/R] is the special unicode (\x2400 range) character for \r.)

Proposed technical implementation details (optional)

Add an optional 2nd debug option after the "debugFeatures": true, option.
Name the option: "debugFeature_Show_NL": true.

@eabase eabase added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Feb 1, 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 Feb 1, 2022
@eabase eabase changed the title Add an additional debug option to actually do a \n when that character is received Add an additional debug option to show a \n when \r is received Feb 1, 2022
@zadjii-msft
Copy link
Member

I'm tempted to just outright reject this - I actually quite like that every character that's emitted to the debug tap is turned into a printable character. It gets rid of any guesswork as to whatever the line discipline is supposed to be. You can just look at the output and know "yep, there was the CR, there's the NL, the BS SPC BS", etc.

@DHowett is the father of the debug tap so I'll give him the final say. If anything, the setting should probably be debugFeature.showNewlines, since we're sorta using the dot notation for things like that

@eabase
Copy link
Author

eabase commented Feb 1, 2022

The problem is only of readability, because when you are doing something on CLI/prompt, the debug output is always misaligned, so it's very hard to read where was the last place something was output/input. So being able to add a real newline where there are, either an \n or a \r, in addition to the unicode block symbol for that, would really help the readablity and alignment. Everything else is great and remain the same. The symbols are all there, only occasionally separated by a visible newline.

An additional advantage is that of copy pasting the debug output. If you don't have a visible \n (or \r) you will get the entire copied part, in one very long string. Not very practical for reporting and including in bug reports.

@j4james
Copy link
Collaborator

j4james commented Feb 1, 2022

I don't feel strongly about this one way or the other, but if we were going to have this feature, I'd expect it to be trigger by a \n rather than a \r, and I'd also expect the line break in the log to come after the output of the control character. For example, something like this:

C:\>␛[13;28;13;1;0;1_␛[13;28;13;0;0;1_␍␊
C:\>

This is similar to what you'd see if you've enabled visible whitespace in a text editor.

@eabase
Copy link
Author

eabase commented Feb 1, 2022

This is similar to what you'd see if you've enabled visible whitespace in a text editor.

Even better! 👍

@zadjii-msft zadjii-msft added Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface 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. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Feb 17, 2022
@zadjii-msft zadjii-msft added this to the Icebox ❄ milestone Feb 17, 2022
@zadjii-msft zadjii-msft added Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Feb 17, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 17, 2022
@DHowett
Copy link
Member

DHowett commented Jun 29, 2022

Honestly, I think we should probably just do this all the time and without a setting. Right now, the debug tap is incapable of producing a physical CR/LF, so adding one would be adding valuable clarifying whitespace without adding confusion as to where that whitespace came from.

Thanks for the request, and I'm sorry I missed this thread the first time!

@ghost ghost added the In-PR This issue has a related PR label Jul 10, 2022
@ghost ghost closed this as completed in #13475 Jul 11, 2022
ghost pushed a commit that referenced this issue Jul 11, 2022
## Summary of the Pull Request

When the debug tap converts control characters into visible glyphs, it
ends up losing the structure of the output, and that can sometimes make
things difficult to read. This PR attempts to alleviate that problem by
reinjecting an actual line break in the debug stream whenever an `LF`
control is received.

## PR Checklist
* [x] Closes #12312
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #12312

## Validation Steps Performed

I've tested the updated debug tab with a number of different shells, and
also a couple of different apps. When there aren't many linefeeds in the
output, it's obviously not going to make much of a difference, but when
there are, I think it definitely improves the readability.
@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 Jul 11, 2022
DHowett pushed a commit that referenced this issue Jul 15, 2022
## Summary of the Pull Request

When the debug tap converts control characters into visible glyphs, it
ends up losing the structure of the output, and that can sometimes make
things difficult to read. This PR attempts to alleviate that problem by
reinjecting an actual line break in the debug stream whenever an `LF`
control is received.

## PR Checklist
* [x] Closes #12312
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #12312

## Validation Steps Performed

I've tested the updated debug tab with a number of different shells, and
also a couple of different apps. When there aren't many linefeeds in the
output, it's obviously not going to make much of a difference, but when
there are, I think it definitely improves the readability.

(cherry picked from commit 04478d1)
Service-Card-Id: 84116394
Service-Version: 1.14
DHowett pushed a commit that referenced this issue Jul 19, 2022
## Summary of the Pull Request

When the debug tap converts control characters into visible glyphs, it
ends up losing the structure of the output, and that can sometimes make
things difficult to read. This PR attempts to alleviate that problem by
reinjecting an actual line break in the debug stream whenever an `LF`
control is received.

## PR Checklist
* [x] Closes #12312
* [x] CLA signed.
* [ ] Tests added/passed
* [ ] Documentation updated.
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. Issue number
where discussion took place: #12312

## Validation Steps Performed

I've tested the updated debug tab with a number of different shells, and
also a couple of different apps. When there aren't many linefeeds in the
output, it's obviously not going to make much of a difference, but when
there are, I think it definitely improves the readability.

(cherry picked from commit 04478d1)
Service-Card-Id: 84116395
Service-Version: 1.15
@ghost
Copy link

ghost commented Aug 5, 2022

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

Handy links:

@ghost
Copy link

ghost commented Aug 5, 2022

🎉This issue was addressed in #13475, which has now been successfully released as Windows Terminal Preview v1.15.200.: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-TerminalConnection Issues pertaining to the terminal<->backend connection interface 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