glamour icon indicating copy to clipboard operation
glamour copied to clipboard

Incorrect reference to default word wrap 100 (actually 80)?

Open mmrwoods opened this issue 1 year ago • 2 comments

Hi,

There is a reference to a default word wrap limit of 100 in the following custom renderer example: https://github.com/charmbracelet/glamour/blob/e70ff2d969da09440de65385f992abd31a2a150e/examples/custom_renderer/main.go#L13

This no longer seems to be true, it was only implied by the wrapping library in use at the time, go-wordwrap, but that was replaced with reflow a long time ago, and an explicit 80 character limit seems to have been set in commit 71e84251b38763ab2dc70e2067aee8562090a7b0.

Have I got that right or am I reading the code wrong?

Assuming I've got this correct, while a very minor issue, it has led to confusion on a couple of github cli issues, one referencing the 100 character limit stated in the example custom renderer (https://github.com/cli/cli/issues/1446#issuecomment-666090075), another referencing 80 characters as the limit based on observation (https://github.com/cli/cli/issues/3501#issuecomment-1199500918)

I'm happy to submit a PR for this, but I'm not sure what solution is appropriate. Personally, I'd like to see the default documented in the readme (even by simply adding a comment to the example code for a custom renderer), and the reference to the limit removed from the example, as any errors on the readme are more likely to be noticed and corrected.

mmrwoods avatar Aug 01 '22 11:08 mmrwoods

I think you're right and we did change the default to 80. I like your suggestion: we should document the default in the README and just drop it from the example.

muesli avatar Aug 01 '22 13:08 muesli

Cool, thanks for confirming I was not going mad :-)

I've added a PR https://github.com/charmbracelet/glamour/pull/162 with a small change for this.

mmrwoods avatar Aug 01 '22 13:08 mmrwoods