[RFC] Adding editorconfig for clang-tidy documentation

Clang Frontend clang-tidy
1

Hi, I’m proposing to add an .editorconfig file to standardize formatting rule for RST files in clang-tidy’s documentation. The goal is to reduce formatting drift (80 chars limit, trailing whitespaces, newline at EOF) and make the life of developers easier.

Motivation

In the existing clang-tidy documentation, there are a lot of files with trailing whitespace and lines longer than 80 characters. This cause inconsistent style issues. While code (Python/C++ etc) has clear formatting tools like clang-format, the RST documentation currently lacks an equally lightweight automatic baseline.

By introducing EditorConfig, we can define a baseline set of formatting rules that most modern editors/IDEs can directly (or with a plugin) adapt to and find these formatting problem early.

Proposed Implementation

Add an .editorconfig file under clang-tools-extra/docs/clang-tidy/checks/

[*.rst]
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
end_of_line = lf
indent_style = space
max_line_length = 80

[list.rst]
max_line_length = unset

Also, as suggested here, adding a similar file under clang-tools-extra/clang-tidy/ can help reduce the amount of newline at EOF issues:

[*]
charset = utf-8
end_of_line = lf
insert_final_newline = true
max_line_length = 80

Question

  • Is it feasible to add 3rd party configs in the repo? (There is already a .editorconfig in the LLVM codebase: link here)
  • Any other concerns/suggestions?

Any feedback is appreciated.

2

LGTM. Personally I would’ve just submitted the PR that added the file :)

3

This cause inconsistent style issues. While code (Python/C++ etc) has clear formatting tools like clang-format , the RST documentation currently lacks an equally lightweight automatic baseline.

Since EditorConfig can not be run in CI, it can’t be used as a baseline because baseline should have a stable reproducible run not on GitHub runners. We should have other .rst linters (like doc8 you suggested) chosen as a baseline because the can be run in CI. Or maybe EditorConfig can “run” in CI somehow?

We have max_line_length = 80 in config for .rst files, will it take into account long links?

Also, as suggested adding a similar file under clang-tools-extra/clang-tidy/ can help reduce the amount of newline at EOF issues:

After [clang-tidy][NFC] Add clang-format option to insert newline at EOF by vbvictor · Pull Request #167950 · llvm/llvm-project · GitHub was merged, I think clang-format will be able to handle all of 4 setting you suggested for clang-tools-extra/clang-tidy/ dir.

4

Any other concerns/suggestions?

It may be hard for newcomers to discover and actually apply this config, because VSCode and other editors need extension for EditorConfig to work, so “out of the box” it may not work. So I think we will need to write instructions on getting involved page of how to set up EditorConfig to help write documentation.

Considering the above facts, I suggest we explicitly write preferences in doc style on getting involved page and place an example of EditorConfig with instructions there. Also, include a suggestion that developers could just set up this style in any preferred editor directly without 3rd party configs.

1 Like
5

I don’t think it’s prohibited directly, but I’d mildly stand on a position to not place configs in repo unless we explicitly need them for CI or Infrastructural reasons.
In general, placing configs in repo could lead to argues like: “we already have a config for .vscode X tool, could we also place a config for .nvim Y tool and .idea Z tool”. So I’d prefer to only keep essentials and let the developers decide on how to set up their environment.

So I’d like to gather other opinions from clang-tidy Maintainers side before pushing configs to main.

6

Thanks for your suggestions! I’ll implement the doc change in getting involved page soon :slight_smile:

7

We have max_line_length = 80 in config for .rst files, will it take into account long links?

Unfortunately, I think it isn’t smart enough to handle situations like this.