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

Doc attribution: make user_doc to work with predefined consts. #14001

Open
comphead opened this issue Jan 3, 2025 · 7 comments · May be fixed by #14086
Open

Doc attribution: make user_doc to work with predefined consts. #14001

comphead opened this issue Jan 3, 2025 · 7 comments · May be fixed by #14086
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@comphead
Copy link
Contributor

comphead commented Jan 3, 2025

          I'll file a ticket to make `user_doc` to work with predefined consts.

Currently the doc_section attribute must match fully the predefined DocSection consts, for example

    pub const DOC_SECTION_REGEX: DocSection = DocSection {
        include: true,
        label: "Regular Expression Functions",
        description: Some(
            r#"Apache DataFusion uses a [PCRE-like](https://en.wikibooks.org/wiki/Regular_Expressions/Perl-Compatible_Regular_Expressions)
regular expression [syntax](https://docs.rs/regex/latest/regex/#syntax)
(minus support for several features including look-around and backreferences).
The following regular expression functions are supported:"#,
        ),
    };

In case of doc_section attribute contains any mismatches such function will be silently ignored.
Example

    doc_section(
        label = "Regular Expression Functions"

In the example the description doesn't match causing the function documentation to be ignored

I believe we can make doc macros more smart like:

  • the doc_section can be just a string
  • using the string find correspondent const from scalar_doc_sections.doc_sections()
  • in datafusion/macros/src/user_doc.rs when constructing the builder use the const instead of building DocSection manually

Originally posted by @comphead in #13919 (comment)

@comphead comphead added documentation Improvements or additions to documentation good first issue Good for newcomers labels Jan 3, 2025
@ding-young
Copy link
Contributor

take

@comphead
Copy link
Contributor Author

comphead commented Jan 8, 2025

Hi @ding-young are you still planning to work on this issue?

@ding-young
Copy link
Contributor

ding-young commented Jan 9, 2025

@comphead Yes, I've taken a brief look at scalar_doc_sections and the user_doc macro, and I plan to make the necessary code changes this weekend. If you need this change earlier, please tell me.

@Chen-Yuan-Lai
Copy link
Contributor

@comphead if the doc_section is a string, user_doc macro syntax will change for all functions?

Before

#[user_doc(
    doc_section(label = "String Functions"),
    description = "...",
    syntax_example = "...",
    ...
)]

After

#[user_doc(
    doc_section = "String Functions",
    description = "...",
    syntax_example = "...",
    ...
)]

@ding-young
Copy link
Contributor

ding-young commented Jan 9, 2025

I think we can make the parser to try both,
(1) try to find all these attributes

#[user_doc(
    doc_section(include=true, label = "String Functions", description: None),
    description = "...",
    syntax_example = "...",
    ...
)]

(2) if it fails look for single string like doc_section = "String Functions"

#[user_doc(
    doc_section = "String Functions",
    description = "...",
    syntax_example = "...",
    ...
)]

Thereby we can keep original syntax.
However, I'm curious whether we should replace (1) with (2), or make parser try both.

@comphead
Copy link
Contributor Author

comphead commented Jan 9, 2025

I think folks lets keep it just

doc_section(label = "String Functions"),

where label corresponds to label in predefined doc sections
in this case we avoid massive changes for all builtin functions again.

I understand that we slightly overcomplicate the doc section so it looks like a complex object but in fact it is just a string but if we want to improve it we can do it later

@Chen-Yuan-Lai
Copy link
Contributor

Agree, thanks @comphead @ding-young

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 good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants