Skip to content

chore: add .editorconfig#594

Merged
LecrisUT merged 6 commits intospglib:developfrom
e-kwsm:editorconfig
Aug 15, 2025
Merged

chore: add .editorconfig#594
LecrisUT merged 6 commits intospglib:developfrom
e-kwsm:editorconfig

Conversation

@e-kwsm
Copy link
Copy Markdown
Contributor

@e-kwsm e-kwsm commented Jun 28, 2025

@atztogo atztogo requested review from LecrisUT and lan496 June 28, 2025 03:41
@atztogo
Copy link
Copy Markdown
Collaborator

atztogo commented Jun 28, 2025

I have no idea but am fine for it.

@lan496
Copy link
Copy Markdown
Member

lan496 commented Jul 2, 2025

Can you describe this PR's motivation? I think format rules for C, C++, Fortran, and Python are already in .pre-commit-config.yaml and applied in every commit

@e-kwsm
Copy link
Copy Markdown
Contributor Author

e-kwsm commented Jul 2, 2025

Different users may have different settings for indentation and line length; a formatter enforces the style, but it may bring big diffs, which will be reduced by editorcondfig as far as user’s editor supports it.

And I found that currently clang-format and ruff do not modify EOL, CR vs CRLF, which is configurable by:

@LecrisUT
Copy link
Copy Markdown
Collaborator

LecrisUT commented Jul 2, 2025

I like the idea of this project, but it does overlap with pre-commit, specifically the EOL and CRLF is enforced by

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer

We are missing mixed-line-ending though, could you open one enforcing LF line-ending?

I would suggest punting this in favor of specialized tools that do overall formatting until it can be used for general language settings 1.

Ideas for Domain-Specific Properties

The following properties are not intended to be implemented by EditorConfig.
This is simply a brainstorm of domain-specific properties that could be supported by some tools that rely on EditorConfig files.

There is a proposal to "namespace" properties with a language-dependent prefix. See Issue #332.

For now the IDEs should have support for ruff and clang-format which are our main formatters. I did notice that we are missing badges for our styles, would be nice if we can get a PR to advertise badges for pre-commit, ruff, clang-format that we use.

Footnotes

  1. https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#ideas-for-domain-specific-properties

@e-kwsm
Copy link
Copy Markdown
Contributor Author

e-kwsm commented Jul 2, 2025

I am using neovim, which now natively supports EditorConfig but does not load configs from .clang-format etc unfortunately. (maybe there exists some plugins)
I believe EditorConfig is simple and universal, and complicated styles can be achieved by the specific formatters.

That’s why I opened this PR for less surprising.

Copy link
Copy Markdown
Collaborator

@LecrisUT LecrisUT left a comment

Choose a reason for hiding this comment

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

I thought that neovim would be reading the other files, but I guess not. Well from my view I'm fine with adding these, let's just make sure everything is consistent. If they manage to make the domain-specific parts work, it would be good to push the other tools to read this also.

@atztogo
Copy link
Copy Markdown
Collaborator

atztogo commented Jul 14, 2025

@LecrisUT and @lan496 , can this PR be merged?

Copy link
Copy Markdown
Collaborator

@LecrisUT LecrisUT left a comment

Choose a reason for hiding this comment

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

LGTM now. We should revisit it as the tool evolves.

I wonder why I can't approve the workflow to run the CI. @e-kwsm did you rebase the PR?

@atztogo
Copy link
Copy Markdown
Collaborator

atztogo commented Aug 8, 2025

Can I merge @e-kwsm and @LecrisUT?

@LecrisUT
Copy link
Copy Markdown
Collaborator

LecrisUT commented Aug 8, 2025

Can I merge @e-kwsm and @LecrisUT?

The pre-commit is failing because the database file is not skipped. We need to add a setting in .editorconfig to skip those for now. Once the pre-commit passes, yes we can merge

@LecrisUT
Copy link
Copy Markdown
Collaborator

@e-kwsm I have made some changes to minimize the diffs, can you check if everything looks fine to you as well.

@atztogo
Copy link
Copy Markdown
Collaborator

atztogo commented Aug 15, 2025

Is it ready to merge?

@LecrisUT LecrisUT merged commit c159731 into spglib:develop Aug 15, 2025
43 checks passed
@e-kwsm e-kwsm deleted the editorconfig branch August 15, 2025 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants