IDL Doc Field

[ebrightfield]

Adds a new, optional doc field to several of the IDL structs. The program module, instruction functions, and account structs are searched for inner doc comments (this is intended to minimize intrusion on existing programs, which likely use outer doc comments on those elements). IxArg and IdlField also will grab their associated doc comment.

The doc field is not included in the IDL JSON in cases when it parses to None.

Todo:

• Someone already got close with adding a docs field to the AccountField, it just needs wiring up.
• Considering a command-line flag to suppress inclusion of doc fields in anchor idl parse.

[paul-schaaf]

hey @ebrightfield I would like to get this into the next release. Do you have time to finish this or do you want me to take over?

• We can include outer comments in the IDL too. But we should include a feature like tom did in his PR lang: Parse instruction doc comments into description fields in IDL #1546 I suggest we make this an anti-feature though. So it's enabled by default and the feature turns it off.
• agree that a flag to supress docs in anchor idl parse would be good
• Please also have a look at tom's PR regarding the tests

[ebrightfield]

@paul-schaaf Awesome! I can knock it out tomorrow.

To reiterate, action items are:

• include outer doc comments
• put it behind an (anti-)feature flag
• --no-docs flag in anchor idl parse
• additional testing in the fashion of Tom's PR lang: Parse instruction doc comments into description fields in IDL #1546

[paul-schaaf]

great!

To reiterate, action items are:

yup!

thanks! @ebrightfield

[ebrightfield]

@paul-schaaf just took care of everything, or nearly. Unsure how best to automate testing the anchor idl parse --no-doc, and turning on/off the no_docs feature (they work when I manually test). The rest of the feature has accompanying Typescript tests in the same fashion as Tom's PR.

Also, took the liberty of skipping over the CHECK doc strings during parsing.

[paul-schaaf]

On second thought, I think we don't need the feature.

It seems like features are most useful if your code changes depending on whether your code would change depending on whether it's turned off or on. This isn't the case here, so I think that having a flag on the build command is good enough.

[paul-schaaf]

Also, took the liberty of skipping over the CHECK doc strings during parsing.

sounds good!

[ebrightfield]

Okay, feature removed.

Any ideas on how best to automate test of anchor idl parse --no-doc?

Also, should --no-doc also be an option on anchor build?

[paul-schaaf]

Also, should --no-doc also be an option on anchor build?

yes that is what I meant by flag on build

Any ideas on how best to automate test of anchor idl parse --no-doc?

Manual testing is fine. Not critical if it doesn't work.

• I think it would be easier to parse the idl doc field if it was an array of the lines instead of a space separated list
• also pls move the tests into their own program inside the misc workspace and have them run as a separate test suite. Once you merge master and fix the conflicts, you'll be able to see 2 test suites in misc already that you can check out for how to do it

[ebrightfield]

Okay, --no-doc added to build and publish commands. And doc fields are Option<Vec<String>> instead of Option<String> now.

After merging, I created a dedicated program for testing under tests/misc/programs/idl_doc, and I emulated the structure of the rest of the misc tests on master, although I am too much of a newbie with JS tooling and could not figure out how they're supposed to get executed.

Edit: Got tests working. Should be good to go.

[paul-schaaf]

what's the need for --no-doc for publish? I can see there's a call to build in the publish code but I think thats only used to check whether compilation succeeded, so it doesn't matter if there are docs generated or not.

[ebrightfield]

Ah, got it. Wasn't sure about that one. Just removed it from the publish command.

[paul-schaaf]

looks like there already has been code to parse docs before your PR that was saved in this docs var. That code can be deleted now and the var too.

Although I think docs is the more common name so it would make sense to me to use docs everywhere instead of doc

[ebrightfield]

Sounds good, renamed to docs across the board (including no_doc --> no_docs where applicable.

Removal of the older docs var caused a few compile errors in lang/syn/src/codegen/accounts/__*.rs. Unsure how best to proceed on those. Appears to be formatting the string back into source code.

Edit: For now, I just decided to convert the is_empty check to an if let Some(docs) = s.docs, and construct the TokenStream by iterating over the doc strings.

[paul-schaaf]

@ebrightfield is there anything else that youre about to push or can I merge now?

[ebrightfield]

I'm satisfied, go for it.

[paul-schaaf]

thanks for the contribution!