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

wordwrap should reset and restore ansi at linebreak #3

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

treilik
Copy link

@treilik treilik commented Oct 7, 2020

ansi-sequence should end at linebreak and start again after it
if there is more text, in the next line.

@lukasmalkmus
Copy link
Contributor

@treilik Not sure you're aware of this, but the test fails in CI.

@treilik
Copy link
Author

treilik commented Oct 25, 2020

Thank you @lukasmalkmus, I am aware ;)
I first changed the test, to what i think, should be the behavior of wordwrap and now i'am still working on the change, to satisfy the test I have changed.

@lukasmalkmus
Copy link
Contributor

Ups, yes. Should have just looked at your changes :)

Looking forward to your solution :)

@treilik treilik force-pushed the master branch 2 times, most recently from 81e21bd to fecab68 Compare October 26, 2020 21:14
@treilik
Copy link
Author

treilik commented Oct 27, 2020

@lukasmalkmus here you go ;)

If you have some thoughts on this, i would love to here them!

@lukasmalkmus
Copy link
Contributor

Looks good to me, but I'm not that deep into the materia :) Make sure to fix the linter issues which are annotated in the Changes view and probably ask @muesli for a review :)

@treilik
Copy link
Author

treilik commented Oct 29, 2020

Didn't noted the linter issues, so i will get to it. Thank you :)

@muesli
Copy link
Owner

muesli commented Oct 29, 2020

@treilik Whenever you consider this ready for a review, feel free to remove the Draft flag!

@treilik treilik force-pushed the master branch 2 times, most recently from ed15216 to aca706d Compare November 1, 2020 21:07
@treilik treilik marked this pull request as ready for review November 3, 2020 10:35
@treilik
Copy link
Author

treilik commented Nov 15, 2020

Hey @muesli, if you could spare the time, i would now be happy for a review :)

@craftamap
Copy link

Hey there - is there any progress here?
would be nice to get this soon - otherwise my CLI-application craftamap/bb has some rendering issues

@treilik treilik force-pushed the master branch 2 times, most recently from 6f6f884 to 7541e7e Compare March 5, 2021 11:56
@treilik
Copy link
Author

treilik commented Mar 11, 2021

Hey muesli!
so i would dare to say i am finally finished :)
if you would like to first get an overview of the change in behavior, have a look into the diff of the test-file.
and than: Into the code!
The core change is here in line 195 of wordwrap.go you find the case of a rune which overflows the line-limit and thus causes a new line, maybe note also the change in the adding of pending spaces if the line is not long enough for all the spaces: in line 85 of wordwrap.go in this diff.
The later commits are handling linting and some edge-cases like leading and interposed zero ansi-arguments or multiple hyphens.

Cheers!

@treilik treilik force-pushed the master branch 2 times, most recently from f68fdaa to ea699ef Compare September 22, 2021 03:27
treilik added 5 commits September 22, 2021 05:30
ansi-sequence should end at linebreak and start again after
if there is more text, in the next line.
but hardwrap not working yet
for better managing char-preserving and hardwrap
to test space preserving and HardWrap shortcut function
treilik added 3 commits September 22, 2021 05:39
correcting missspell and adding toplevel-comment period
so that the can be HardWrapped and are not ignored.
According Test are added too.
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