Skip to content

Conversation

@Riksu9000
Copy link
Contributor

@Riksu9000 Riksu9000 commented Aug 15, 2022

I'm testing some autoformatters. They all seem to be opinionated and quirky. Here I've compiled the less opinionated changes to begin with.

Remove whitespace at the end of lines.
Remove leading space from unordered lists.
Enforce two newlines between sections.
Always use - for unordered lists.
Slightly tweaked tables.
Indentation fixes.
Escape special characters where necessary.

@Riksu9000 Riksu9000 added the documentation Improvements or additions to documentation label Aug 15, 2022
@Riksu9000 Riksu9000 added this to the 1.11.0 milestone Aug 15, 2022
@Itai-Nelken
Copy link
Contributor

This doesn't seem to be a problem in the files changed, but 2 spaces at the end of a line mean a newline, so they might have been put intentionally.

@JF002
Copy link
Collaborator

JF002 commented Aug 15, 2022

I'm OK with those changes, they'll make our docs more consistent. We'll probably want to specify those conventions in the documentation

Did you run a tool to make those changes automatically? If yes, I think it could be a good idea to document how to use it, provide a configuration file (like we did for clang-format/tidy), and maybe add it in the format workflow for Github Actions.

@Riksu9000
Copy link
Contributor Author

@JF002 Absolutely. I was initially working on adding a markdown formatter check, but none of the formatters I tested produced the exact format I was expecting so I need to experiment some more. These are cherry picked changes which guidelines and autoformatters agree with, so we can focus on the small details later.

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While changing the docs, can we also use the "one sentence one line" rule for markdown. It makes the diffs much more readable, as a single word change won't lead to a whole paragraph diff as the text flows down to the next line

For GitHub we'd then need two spaces at the end of the line to keep the sentences in one paragraph (on GitLab one would need to put a blank line in between sentences to create a new paragraph)


My own contribution is little more than a brute force conversion to
python3. It is sparsely tested so there are likely to be a few
My own contribution is little more than a brute force conversion to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've always kept to the "one sentence one line" rule for markdown. Makes diffs MUCH more readable and formatting won't mess with a small change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to agree, but none of the formatters I tested so far support this. We'll have to discuss this later. This PR isn't intended to fix everything, but only the most obvious issues.

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two minor suggestions, after that I'm very happy with this PR. Thank you for making the docs much more coherent!

Copy link
Collaborator

@JF002 JF002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with those changes. I think it would be a good idea to list those formatting rules in coding-convientions.md because I'll probably be the first one to forget about them ;-)

@Riksu9000 Riksu9000 merged commit c2b6a8d into InfiniTimeOrg:develop Aug 21, 2022
@Riksu9000 Riksu9000 deleted the fix-md-format branch August 21, 2022 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants