Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init support for doc-comments #91

Merged
merged 5 commits into from
Feb 27, 2024

Conversation

hsjobeki
Copy link
Collaborator

@hsjobeki hsjobeki commented Nov 13, 2023

@infinisil

Adds support for doc-comments. (see: NixOS/rfcs#145)

  • support for doc-comments in parallel.
  • Shift all markdown headings down one two levels, so the nixpkgs manual rendering still works.
  • Backwards compatibility with arguments
  • Native compatibilty with structured arguments (referred as 'lambda formals' within rfc145)
  • Unit Tests
  • Documentation & Changelog update

Doesn't discover new items, it just renders, what was rendered before. (e.g. lib.lists.map keeps name and section heading)
But If the content is a new doc-comment (surrounded by /** */) then it doesn't perform the 'nixdoc' rendering with Example: and Type: sections. Instead the section headings are written in explicit markdown by the user/documentation writer.
All section headings are automatically shifted down two levels. (Means you can and should currently write H1 to H4 in your doc comments)

This is certainly a limitation, to resolve this we need to change the nixpkgs manual rendering tool (nixos_render_docs) which could be one of the next steps.

Moved some rust functions into separate files. Having everything in main.rs makes it hard to maintain two separate feature sets.

  • functions that are only needed for legacy support: legacy.rs
  • shared functions that are only needed formatting content: format.rs
  • unit tests: test.rs

Version 3.0.0

This PR bumps the version to 3.0.0

  • Added Changelog Entry
  • Added Migration tutorial
  • Reformulate Readme

@hsjobeki hsjobeki marked this pull request as draft November 13, 2023 16:35
@hsjobeki hsjobeki marked this pull request as ready for review November 14, 2023 10:41
@infinisil
Copy link
Collaborator

infinisil commented Nov 16, 2023

Nice! We should definitely wait for the RFC to be accepted, which should be pretty soon. After that I'm generally in favor of merging it, but we definitely need tests, documentation and a changelog entry for this.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-01-04-documentation-team-meeting-notes-102/37754/1

Co-authored-by: Shahar Dawn Or <mightyiampresence@gmail.com>
@hsjobeki
Copy link
Collaborator Author

@infinisil could i ask you for some review & suggestions?

@infinisil
Copy link
Collaborator

Nice, I've been eagerly awaiting a ping for it to be ready :D. I can't get to it immediately, but I'll take a close look when I get to it!

Copy link
Collaborator

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Overall looking really good!

I'd like to see some extra tests:

  • One to make sure that the headings get indented properly

  • One to make sure omitting a doc comment from an attribute doesn't duplicate the previous one:

    {
      /** Comment */
      foo = 0;
    
      # This should not have any docs
      bar = 1;
    }

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
doc/migration.md Outdated Show resolved Hide resolved
src/format.rs Outdated Show resolved Hide resolved
src/format.rs Outdated Show resolved Hide resolved
src/format.rs Outdated Show resolved Hide resolved
src/format.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-02-15-documentation-team-meeting-notes-109/39826/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-54/39990/1

@hsjobeki
Copy link
Collaborator Author

@infinisil. Just worked your feedback in.

@hsjobeki hsjobeki force-pushed the feat/doc-comment-support branch 2 times, most recently from c0eafe0 to 72742d1 Compare February 25, 2024 12:45
@hsjobeki
Copy link
Collaborator Author

@infinisil to keep you updated.
I just bootstrapped a cli tool https://github.com/hsjobeki/flutsch. Which in combination with a static analysis tool could replace nixdoc entirely.

Features such as #98 make more sense on nix value level. Unfortunately the comments are not part of the c++nix ast which requires invoking an addtional (e.g. rnix based) tool.

Copy link
Collaborator

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Just some minor bits now

I just bootstrapped a cli tool https://github.com/hsjobeki/flutsch. Which in combination with a static analysis tool could replace nixdoc entirely.

Nice! I generally don't mind archiving nixdoc if there's something better. Though I definitely can't say I'm a fan of C++ 😅

src/format.rs Outdated Show resolved Hide resolved
test/headings.md Outdated Show resolved Hide resolved
@hsjobeki
Copy link
Collaborator Author

Just some minor bits now

I just bootstrapped a cli tool https://github.com/hsjobeki/flutsch. Which in combination with a static analysis tool could replace nixdoc entirely.

Nice! I generally don't mind archiving nixdoc if there's something better. Though I definitely can't say I'm a fan of C++ 😅

Yes. C++ is horrible.
My vision is to use it only for the information that we need to pull out of the native evaluator. (Using rust with the tvix evaluator would be a nice alternative but we wouldn't want nixpkgs docs to depend on tvix^^)

Then using a static rust/rnix based tool on top of that (introspection information) to generate the markdown docs.

Co-authored-by: Silvan Mosberger <github@infinisil.com>
@infinisil infinisil merged commit 31ff6f6 into nix-community:master Feb 27, 2024
2 checks passed
@hsjobeki hsjobeki deleted the feat/doc-comment-support branch February 28, 2024 10:26
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