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

Clear all lines during Interject_with #30

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

Gbury
Copy link
Contributor

@Gbury Gbury commented Nov 16, 2022

This should solve #29 .

After looking a bit at the code, it seems like the code for interject_with only clears the line once, after moving up, so it effectively only clears the topmost line. This PR tries to clear all the lines, by clearing the current line, and then moving up, one line at a time.

On my example, it seems to fix the problem.

@Gbury
Copy link
Contributor Author

Gbury commented Jul 18, 2023

I have rebased this PR. Is there anything I should do to get this merged ?

@tmcgilchrist
Copy link

This would be a useful bug fix to include, I have encountered the same issue in my use of progress.
Could it be merged @dinosaure ?

@Gbury
Copy link
Contributor Author

Gbury commented Jan 30, 2024

Ping ? This one bug has been preventing me from using progress in a projet of mine for more than a year because one cannot use both multiple lines and pauses/interject_with without thrashing the output.

@tmcgilchrist
Copy link

cc @craigfe

@dinosaure
Copy link
Collaborator

It seems right, I've also observed your behaviour and this patch should correct it. I'm going to integrate this PR but we're definitely lacking tests that can help us better specify the behaviour of progress.

@dinosaure dinosaure merged commit ac53cd4 into craigfe:main Jan 31, 2024
1 check passed
dinosaure added a commit to dinosaure/opam-repository that referenced this pull request Apr 13, 2024
CHANGES:

- Be compatible with MirageOS and remove `ocaml_terminal_get_sigwinch` (@art-w, craigfe/progress#38)
- Clear all lines in `interject_with` (@Gbury, craigfe/progress#30)
- Add `Display.remove_line` (@mbarbin, craigfe/progress#26)
- Fix compilation for OCaml 5.2 (reported by @Gbury, fixed by @dinosaure, craigfe/progress#40)
- Add `Display.{pause,resume}` (@Gbury, craigfe/progress#37)
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.

3 participants