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

Expose a ClearScreen method #37

Merged
merged 2 commits into from
Nov 7, 2023
Merged

Expose a ClearScreen method #37

merged 2 commits into from
Nov 7, 2023

Conversation

slingamn
Copy link
Member

@slingamn slingamn commented Nov 5, 2023

For parity with the original library; fixes #36.

cc @wader @dominikschulz

We no longer need platform-specific screen-clearing code, as per #2. Some of the other platform-specific utilities may be obsolete as well...

I tested this on Linux and Windows with the readline demo, and on Linux with gopasspw/gopass#2684 using replace in go.mod. Everything looks OK so far.

For parity with the original library; fixes #36
Copy link

@dominikschulz dominikschulz left a comment

Choose a reason for hiding this comment

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

Thanks a lot. This is great :)

example/readline-demo/readline-demo.go Show resolved Hide resolved
@@ -304,6 +302,11 @@ func (i *Instance) EnableHistory() {
i.operation.history.Enable()
}

// ClearScreen clears the screen.
func (i *Instance) ClearScreen() {

Choose a reason for hiding this comment

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

I'm not very familiar with the internals of this library, but in my experiments I had to call o.Clean() and o.Refresh() as well. Should this be part of the ClearScreen method?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you write to (*Instance).Stdout() or similar, it will refresh automatically via wrapWriter:

func (w *wrapWriter) Write(b []byte) (int, error) {

@@ -38,11 +37,6 @@ func GetScreenSize() (width int, height int) {
return
}

// ClearScreen clears the console screen
func ClearScreen(_ io.Writer) error {
return SetConsoleCursorPosition(&_COORD{0, 0})
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was weirdly named?

@@ -74,6 +75,11 @@ func (r *rawModeHandler) Exit() error {
return err
}

func clearScreen(w io.Writer) error {
_, err := w.Write([]byte("\x1b[H\x1b[J"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe at some point named constant etc for ansi stuff could be nice for readability?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, maybe. What I've found in the past is that the composition rules are tricky. ircdog used to use mgutz/ansi --- I ended up removing it and handcrafting my own ANSI codes because I found that it obscured more than it clarified.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Aha i see, yeah i actually ran into something similar when trying to general ANSI-aware line wrapper for fq :)

@slingamn slingamn merged commit cf96f85 into master Nov 7, 2023
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.

Where is the ClearScreen method?
3 participants