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

When wrapping text, the first line of a subsequent paragraph is wrong. #1066

Closed
pwhipp opened this issue Jul 10, 2018 · 5 comments · Fixed by #1478
Closed

When wrapping text, the first line of a subsequent paragraph is wrong. #1066

pwhipp opened this issue Jul 10, 2018 · 5 comments · Fixed by #1478
Labels
Milestone

Comments

@pwhipp
Copy link

pwhipp commented Jul 10, 2018

In click 6.7, python 3.6.4

This help text for an option:
'x\n\nI am going to get broken in the wrong place.\n'

Looks fine:

  --test TEXT                     x
                                  
                                  I am going to get broken in the wrong
                                  place.

But if we make our first paragraph last line a bit longer:
'This will break the next paragraph.\n\nI am going to get broken in the wrong place.\n'

The line position for the first paragraph is carried forward incorrectly when wrapping the first line of the following paragraph so it breaks the line in the wrong place:

  --test TEXT                     This will break the next paragraph.
                                  
                                  I am
                                  going to get broken in the wrong place.

The result should be:

  --test TEXT                     This will break the next paragraph.
                                  
                                  I am going to get broken in the wrong
                                  place.
@chrahunt
Copy link

chrahunt commented Nov 22, 2018

Also noticed with Click 7.0 Python 3.7.

@jcrotts jcrotts added the bug label May 5, 2019
@Zarad1993
Copy link

Zarad1993 commented May 6, 2019

Looking at this.

@jcrotts jcrotts added this to the 8.0 milestone May 6, 2019
@Zarad1993
Copy link

Zarad1993 commented May 6, 2019

****** Debugging Notes *******
There is a certain width set in here,

text_width = max(self.width - first_col - 2, 10)

If you try setting the width to 100. This is the result

Screen Shot 2019-05-06 at 3 52 51 PM

Otherwise, that is an existing bug.
master:

Screen Shot 2019-05-06 at 3 53 40 PM


I have a fix in mind for this that is backward compatible and does not affect the usage.
The fix in mind is to only check if new lines (\n) come in text. Recalibrate the width based on each line and not on all the lines.

@chrahunt ☝️

@jcrotts
Copy link
Member

jcrotts commented May 6, 2019

I like your proposed fix, although I'm not sure why the original formatting values were chosen. I'm sure David would be happy to review the PR.

Zarad1993 added a commit to Zarad1993/click that referenced this issue May 7, 2019
This commit addresses the issue with wrong breakage
when trying to wrap text with already given broken
lines. This was failing due to the text width, as we expandtabs
on every line. The width exceeds the limits and breaks. Does not
really break on the given (\n), rather it breaks when the width passes.
The fix in this commit checks if new line was introduced in a the description.
Then splits those into readable chunks and wrap the text based on each line instead
of wrapping up the text on all lines together which will end up reading the wrong
text width.
Zarad1993 added a commit to Zarad1993/click that referenced this issue May 7, 2019
This commit addresses the issue with wrong breakage
when trying to wrap text with already given broken
lines. This was failing due to the text width, as we expandtabs
on every line. The width exceeds the limits and breaks. Does not
really break on the given (\n), rather it breaks when the width passes.
The fix in this commit checks if new line was introduced in a the description.
Then splits those into readable chunks and wrap the text based on each line instead
of wrapping up the text on all lines together which will end up reading the wrong
text width.
@Zarad1993
Copy link

Zarad1993 commented May 7, 2019

@jcrotts - I've got a PR up #1307 . I'll be happy to work on the changes required if needed.

Thanks

@davidism davidism modified the milestones: 8.0, 7.1 Feb 22, 2020
@davidism davidism linked a pull request Feb 23, 2020 that will close this issue
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants