Skip to content

Precommit: Format all Markdown files - #8201

Closed
Growl1234 wants to merge 1 commit into
deepmodeling:developfrom
Growl1234:precommit-md
Closed

Growl1234 wants to merge 1 commit into
deepmodeling:developfrom
Growl1234:precommit-md

Conversation

@Growl1234

Copy link
Copy Markdown
Collaborator

Two notes:

  • Ordered lists: By default, mdformat uses 1. / 1. / 1. instead of 1. / 2. / 3.. I think this is a sensible default and am reluctant to change it. It avoids unnecessary numbering changes in diffs when list items are added, removed, or reordered. The rendered lists are still numbered consecutively as 1 / 2 / 3.
  • Line wrapping: I intentionally chose a target line width of 100 columns to improve readability in plain terminals and text editors without automatic line wrapping. This is the only formatting setting I changed from its default. Under CommonMark (including MyST), ordinary line breaks within a paragraph are treated as soft breaks, while blank lines separate paragraphs. Therefore, wrapping source lines does not change how paragraphs appear in the rendered HTML/PDF output, and no text is truncated.

@Growl1234 Growl1234 changed the title Precommit: Prettify add Markdown files Precommit: Prettify all Markdown files Oct 22, 2026
@Growl1234 Growl1234 added Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS The Absolute Zero Reduce the "entropy" of the code to 0 labels Oct 22, 2026
@Growl1234 Growl1234 closed this Oct 22, 2026
@Growl1234 Growl1234 reopened this Oct 22, 2026
@Growl1234 Growl1234 changed the title Precommit: Prettify all Markdown files Precommit: Format all Markdown files Oct 22, 2026

@ABACUS-PR-BOT ABACUS-PR-BOT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Static review of the complete diff at c7d34121745b, independently checked against the configured Markdown/MyST rules. The comments cover four documentation/configuration regressions and one non-blocking link suggestion; no documentation build, formatter hook, or ABACUS runtime was executed.

Comment thread .pre-commit-config.yaml
- id: mdformat
args: [--wrap, "100"]
additional_dependencies:
- mdformat-gfm

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The formatting changes TeX itself: for example, \sum becomes \\sum and subscripts gain escapes in hs_matrix.md, pp_orb.md, and hsolver.md. With dollarmath enabled in docs/conf.py, MyST passes these payloads directly to math nodes, so these are changed operators/subscripts rather than harmless prose escapes. Please restore the original math and use formatting support for this MyST syntax (or exclude unsupported files), checking math-token preservation before reformatting.

---
______________________________________________________________________

## applyTo: "\*\*"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This changes the YAML applyTo: "**" frontmatter into a thematic break and a heading. Copilot reads that glob from frontmatter to select path-specific instructions, so the heading no longer supplies the all-files selector. Please restore the frontmatter and preserve it during formatting, or exclude this instruction file from unsupported formatting.

| number of atoms | 'TOTAL ATOM NUMBER\\s\*=\\s\*(\\d+)' | `TOTAL ATOM NUMBER = 2` |
| lattice constant | 'lattice constant (Angstrom)\\s\*=\\s\*(-?\\d+(.\\d+)?)' | `lattice constant (Angstrom) = 0.529177` |
| lattice vectors | '^Lattice vectors: (Cartesian coordinate: in unit of a_0)$' | ` Lattice vectors: (Cartesian coordinate: in unit of a_0)` |
| coordinate system | '^(DIRECT | CARTESIAN) COORDINATES' |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The escaped alternation pipe has become a table separator: this row now contains the regex fragment '^(DIRECT and puts CARTESIAN) COORDINATES' in the example column, losing the original DIRECT COORDINATES example. The k-point and eigenvalue rows also lose cell contents. Please restore the complete original cells and preserve literal \| inside regex cells when formatting.

@@ -1,10 +1,13 @@
# Migration Guide: New CSR Format for H\(R\) and S\(R\) Matrices
# Migration Guide: New CSR Format for H(R) and S(R) Matrices

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Removing the escapes from H\(R\) and S\(R\) changes their meaning under the existing MyST replacements extension: the title and overview now parse as H® and S®. Please preserve the protected parentheses or put these matrix symbols in math/code spans, and check with the enabled replacements rule.

Comment thread docs/community/cicd.md

- Integration test and unit tests (`test.yml`): This action builds ABACUS with all available
features, runs integration tests, and runs unit tests. It also performs
[static analysis](../CONTRIBUTING.md#code-formatting-style) with \<pre-commit.ci>.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Non-blocking: escaping the opening < removes the pre-commit.ci hyperlink under the enabled MyST linkify rule. Could this use an explicit HTTPS link instead, so the service remains directly reachable from this page? A follow-up after merge is fine.

@Growl1234
Growl1234 marked this pull request as draft October 23, 2026 06:46
@ZhouXY-PKU ZhouXY-PKU removed the The Absolute Zero Reduce the "entropy" of the code to 0 label Oct 23, 2026
@Growl1234 Growl1234 closed this Oct 23, 2026
@Growl1234
Growl1234 deleted the precommit-md branch October 23, 2026 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants