Basic ANSI modes #267
Basic ANSI modes #267
Conversation
ppolesiuk
left a comment
ppolesiuk
left a comment
There was a problem hiding this comment.
Looks good. I have some minor comments regarding documentation and code formatting (see below).
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new ANSI terminal control library that provides escape codes for terminal manipulation. The module includes functionality for cursor movement, screen clearing, text styling, and screen buffering control.
- Introduces the
AnsiTerminalmodule with ANSI escape code constants and functions - Provides organized submodules for cursor control, screen erasing, font/color styling, and buffering
- Implements helper functions that generate ANSI escape sequences for various terminal operations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Copilot wrote some comments that could be addressed. And there is still one typo from the previous review, that is not fixed. |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The longer I look at this PR, the more questionable this module seems to me. I’m not sure the standard library is the right place for it. Yes, I implemented it ahead of time, when I was shaping the pretty-printer, but now that the dust has settled, I’m left wondering whether this whole thing makes for a good design. On the other hand the Testing module also requires part of this functionality. Separate module with definitions would spare us some amount of boilerplate in other modules. But honestly, is it worth it? |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ppolesiuk
left a comment
ppolesiuk
left a comment
There was a problem hiding this comment.
The PR is ready for merge. Thanks!
2 participants
This is part of #256 PR separated for easier review and merging