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

Reorganize std docs so that they follow the same tree structure that the std crate does #5797

Open
jfecher opened this issue Aug 22, 2024 · 2 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@jfecher
Copy link
Contributor

jfecher commented Aug 22, 2024

Problem

Organization of functions & other items in the standard library docs for Noir is arbitrary.

Happy Case

The organization should mimic the actual modules in the std crate itself. This will also give a better idea to users on how the stdlib is structured.

For bonus points these docs could be autogenerated from doc comments on stdlib items.

Workaround

None

Workaround Description

No response

Additional Context

No response

Project Impact

None

Blocker Context

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@jfecher jfecher added the enhancement New feature or request label Aug 22, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Aug 22, 2024
@jfecher jfecher added the documentation Improvements or additions to documentation label Aug 22, 2024
@asterite
Copy link
Collaborator

asterite commented Sep 6, 2024

I'd really like us to have auto-generated docs, and docs when hovering over items.

My idea, to begin with, was this:

  1. We start producing the doc comments in the lexer, but only when needed (if in LSP mode, or if generating docs (which we currently don't have, but...))
  2. We consume doc comments before items that could have doc comments (modules, structs, functions, etc.).
    a. That means that a doc comment in an unexpected position will produce an error... maybe that's fine? It seems Rust works that way too.
  3. We store those doc comments as a vector of strings, unprocessed, likely in NodeInterner
  4. In LSP, when hovering over an item, or when suggesting autocompletion, we fetch any doc comments attached to an item, concatenate them, and show the result as markdown

At least doc comments will start showing up in the editor. Once that's done we can start working on a doc generator (not sure if there's something we could reuse for actually presenting the docs once we process them).

I'd be happy to work on this in parallel to the other stuff 😄

@jfecher
Copy link
Contributor Author

jfecher commented Sep 6, 2024

Sounds good to me. I'm not sure how much work we'd save by only saving doc comments if we're in lsp mode or generating docs but the plan sounds sensible.

github-merge-queue bot pushed a commit that referenced this issue Sep 9, 2024
# Description

## Problem

Part of #5797

## Summary


![lsp-doc-comments](https://github.com/user-attachments/assets/9560d41a-3163-4489-820e-a0f488b98f1a)

This is just an experiment! 

We now parse doc comments, like we parse attributes, and attach them to
AST nodes. From there we store them in `NodeInterner` (for now only if
in LSP mode). Then we retrieve those comments for hover and
autocompletion.

## Additional Context

I decided to always store the doc comments in the NodeInterner.

I've been thinking and maybe the doc comments shouldn't be parsed as
`String`, but instead be a `Span`. Then we wouldn't need to allocate a
string for them, and only read from the source file when needed. But...
that's an optimization that can be done later.

After merging this, the next steps could be:
1. Copy the doc comments found in the many `.md` files into the standard
library (they will be duplicated, but just for a wihle)
2. Create a script or something that generates the `.md` files from the
doc comments

Note: I removed the lalrpop code because I got some errors and I thought
maybe it's not very useful to keep that code around if we have to make
changes to it to maintain it, but it's not working (we could always
revive it from git's history). But let me know if this is wrong.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Sep 9, 2024
# Description

## Problem

Part of #5797

## Summary

This PR copies the docs from the `.md` file to the `.nr` file.

## Additional Context

I'll be sending one PR per `.md` file. It looks like they are 36 in
total so it will take some time to port all the docs, but not that much.
And this way reviewing these PRs will be more manageable.

For now examples are copied inline into the `.nr` file. Eventually we
could maybe have `nargo test` run or at least type-check these example

## Documentation

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@asterite asterite mentioned this issue Sep 9, 2024
5 tasks
github-merge-queue bot pushed a commit that referenced this issue Sep 9, 2024
# Description

## Problem

Part of #5797

## Summary



## Additional Context



## Documentation

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
github-merge-queue bot pushed a commit that referenced this issue Sep 13, 2024
# Description

## Problem

Part of #5797

## Summary



## Additional Context



## Documentation

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
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 enhancement New feature or request
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants