Skip to content

fix: match display of BEL, ST, DEL with other Ctrl#37

Merged
caarlos0 merged 1 commit intocharmbracelet:mainfrom
mkotowski:fix/missing_ctrl_prints
Dec 2, 2024
Merged

fix: match display of BEL, ST, DEL with other Ctrl#37
caarlos0 merged 1 commit intocharmbracelet:mainfrom
mkotowski:fix/missing_ctrl_prints

Conversation

@mkotowski
Copy link
Copy Markdown
Contributor

Context: This PR should align some minor inconsistencies in how some C0 and C1 characters are displayed.

Modify BEL, ST, and DEL labeling to match other Ctrl sequences:

  • Add missing label for DEL.
  • Fix BEL and C1 ST not having their hex code displayed when not part of a sequence
  • Handle ESC \\ (C0 ST)
  • Add tests for C0, C1, and misc ASCII (space and DEL)
λ echo -ne '\x1f\x7f\x9c\t\n\a\e\\' | ./sequin
Ctrl \x1f: Unit separator
Ctrl \x7f:
Ctrl : String terminator
Ctrl \t: Horizontal tab
Ctrl \n: Line feed
Ctrl : Bell
 ESC \\: TODO: unhandled sequence

Screenshot_20241126_225628

After changes:

λ echo -ne '\x1f\x7f\x9c\t\n\a\e\\' | ./sequin
Ctrl \x1f: Unit separator
Ctrl \x7f: Delete
Ctrl \x9c: String terminator
Ctrl \t: Horizontal tab
Ctrl \n: Line feed
Ctrl \a: Bell
 ESC \\: String terminator

Screenshot_20241126_225649

Modify BEL, ST, and DEL labeling to match other Ctrl sequences:

  - Add missing label for DEL.
  - Fix BEL and C1 ST not having their hex code displayed
    when not part of a sequence
  - Handle ESC \\ (C0 ST)
  - Add tests for C0, C1, and misc ASCII (space and DEL)

Signed-off-by: Michał Kotowski <dev@mkotowski.dev>
@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 12039709520

Details

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 88.026%

Totals Coverage Status
Change from base Build 12020822570: 0.1%
Covered Lines: 669
Relevant Lines: 760

💛 - Coveralls

Copy link
Copy Markdown
Contributor

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing, thank you!

Copy link
Copy Markdown
Member

@aymanbagabas aymanbagabas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you @mkotowski!

@caarlos0 caarlos0 merged commit d200b62 into charmbracelet:main Dec 2, 2024
@caarlos0
Copy link
Copy Markdown
Contributor

caarlos0 commented Dec 2, 2024

Thanks @mkotowski !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants