Wrap long lines in comments in Config.md

[stefanhaller]

Since these are displayed in a fenced code block on github, they aren't auto-wrapped on display, so users have to scroll horizontally to read longer paragraphs.

Addresses #800 (comment).

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for 30169bd1	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (30169bd)	Report Missing	Report Missing	Report Missing
Head commit (db9fb21)	58305	50718	86.99%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#4951)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[stefanhaller]

@rayluo FYI

[rayluo]

I thought the changes would include only splitting long lines, like you did for Config.md and user_config.go in the current PR.

Where will the content inside this config.json be used? Normally, we would not hardcode \n inside raw data. How to wrap those raw data is a job for the component who consumes the raw data; if it can't render the data with proper wrapping, so be it. Just my 2 cents.

[stefanhaller]

I guess you are right.

Neither Config.md nor config.json are manually edited; they are both auto-generated from user_config.go. The process is that we generate the schema (config.json) first, and the comments in user_config.go are converted to schema descriptions automatically. Then we generate Config.md from the schema. If we want the schema descriptions to not contain linefeeds, but the lines in Config.md to be wrapped, then this means we need to automatically wrap the lines.

Initially I didn't want to do that, because I thought that in some cases this automatic wrapping would look bad, and we'd have to retain manual control over the wrapping; for example in bullet point lists (here's one example). But actually, having them wrapped to the beginning of the line isn't so bad.

So I reverted the first commit, and implemented auto-wrapping of Config.md. I do agree it's better; please have another look.

Where will the content inside this config.json be used?

It is used by YML language servers to provide auto-completion and documentation on hover. The YML extension in VS Code does this, and it's extremely useful. And it does show that schema descriptions should be paragraphs, not wrapped lines. For example:

In this hover window, lines will be wrapped automatically when the window is too narrow, like so:

[rayluo]

Yes, the new change looks much better.

Since you described the relationship among these files is "user_config.go -> config.json -> Config.md", I have one more optional suggestion. I believe in a version control system (VCS) tenet that "(1) Everything could be stored in VCS; (2) Artifacts that can be derived from #1 shall NOT be stored in VCS". (And I even build a little tool to do that for github pages - but I digress.) Anyway, I mean if you can somehow generate config.json and then config.md dynamically, then you won't need to store them in the code base, and this entire conversation won't need to happen in the first place.

Again, this is an optional suggestion. You don't have to do it, either in this PR or a new PR. Feel free to merge this PR as-is.

[stefanhaller]

Artifacts that can be derived from #1 shall NOT be stored in VCS

I would normally agree with that, but in this case the artefacts need to be hosted on github: Config.md is used as a poor man's manual substitute (it's linked to from lazygit's Status view), and the schema is referenced by the json schema store so that it can be found automatically by language servers.

Sure, you could argue that it would be better to host them somewhere else, and deploy them to that place as part of making a release, but it's extra work to set this up, and we are lazy. 😄

[rayluo]

Nice!

I would say in general all https://... lines shall not be wrapped, regardless of whether it has the "See" or "Please refer to" prefix, though.

[stefanhaller]

I disagree; if a URL occurs in the middle of a line, I see no reason why we shouldn't break right before it, e.g. here. It's really only the "See https://..." lines that are a problem.

Note that this is not a general-purpose formatter; it is very much tailored to our specific needs, and I expect that we might extend the set of special cases in the future when we run into scenarios that are wrapped badly. We don't currently have "Please refer to" in our documentation, so we don't need a rule for it.

Oh, and btw: this is another reason why it's good to include the generated artefacts in the same commit as the source they are generated from. This allows us to see those badly wrapped cases immediately, rather than only much later when the next release is deployed.

[rayluo]

I disagree; if a URL occurs in the middle of a line, I see no reason why we shouldn't break right before it, e.g. here. It's really only the "See https://..." lines that are a problem.

To clarify, my statement of "https://... lines shall not be wrapped" means do not split one url into two (or more) lines.

If a url occurs in the middle of the line, it is of course OK - and often preferable - to put the url into the next line (such as what your example above shows)

Note that this is not a general-purpose formatter

Fair enough.

[stefanhaller]

do not split one url into two (or more) lines.

Ah, right. Yes, I agree, and no, we don't do that. We only ever break lines at spaces.