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

Spinner breaks my shell cursor. #1585

Closed
c-kruse opened this issue Aug 12, 2024 · 4 comments · Fixed by #1616
Closed

Spinner breaks my shell cursor. #1585

c-kruse opened this issue Aug 12, 2024 · 4 comments · Fixed by #1616
Assignees

Comments

@c-kruse
Copy link
Contributor

c-kruse commented Aug 12, 2024

Describe the bug
When a skupper command with a spinner doesn't complete successfully (usually I get impatient and ^C it) the cursor in my shell disappears and I have to run reset to get it back (though I'm sure there's probably a more clever way to go about it.)

I suspect that it is our usage of the spinner library sending control codes but not cleaning up after ourselves.

How To Reproduce
Steps to reproduce the behavior: Run a long skupper command with a spinner like skupper init and kill the process.

otherwise run this little gizmo.

package main
import (
        "context"
        "fmt"
        "os"
        "os/signal"
        "github.com/skupperproject/skupper/pkg/utils"
)
func main() {
        ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt)
        defer cancel()
        err := fmt.Errorf("nope")
        go func() {
                if err := utils.NewSpinner("kill this process and notice your bash cursor being missing", 20, func() error { return err }); err != nil {
                        fmt.Printf("spinner finished with error: %s\n", err)
                }
        }()
        <-ctx.Done()
        fmt.Println("exiting")
}

Expected behavior
I'd love for it to clean up after itself consistently.

Environment details

  • Skupper CLI: 1.7.3

Additional context
Add any other context about the problem here.

@c-kruse
Copy link
Contributor Author

c-kruse commented Aug 12, 2024

@nluaces you ever notice this?
Screencast from 2024-08-12 10-07-29.webm

@nluaces
Copy link
Member

nluaces commented Aug 12, 2024

@c-kruse I have not! but thank you for reporting this issue, as it has to be fixed as well for V2

@nluaces nluaces self-assigned this Aug 12, 2024
@c-kruse
Copy link
Contributor Author

c-kruse commented Aug 12, 2024

Though this bug is very minor, I think the real root of the issue is probably part of something we should look at more closely for v2: gracefully handling signals.

Aside from leaving shells in inconvenient states, we are probably liable to be leaving whatever transactions we're performing in limbo in some places: even if the consequences are mostly just broken http connections. Glad to volunteer my time of this!

@nluaces
Copy link
Member

nluaces commented Aug 12, 2024

Aside from leaving shells in inconvenient states, we are probably liable to be leaving whatever transactions we're performing in limbo in some places: even if the consequences are mostly just broken http connections.

@c-kruse are you thinking in a bigger scope than this particular issue? I mean: have you spotted more similar scenarios not related with the spinner?

Glad to volunteer my time of this!

Thank you!
I would like to collaborate on this too, but in case you want to start right away, feel free to do it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants