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

Discussion: Reconsider @default tag #5

Open
dgkf opened this issue Feb 6, 2023 · 6 comments
Open

Discussion: Reconsider @default tag #5

dgkf opened this issue Feb 6, 2023 · 6 comments

Comments

@dgkf
Copy link
Collaborator

dgkf commented Feb 6, 2023

This tag is "fake", it's processed within the @typed tag.

Right now the roxytypes package also has a feature that lets you generate a little snippet that describes a parameter's default value. roxytypes has the ability to parse this default value from the package, or use a provided default description using the @default value.

I think this is a cool capability, but it probably shouldn't be coupled to the @typed tag. You could just as readily want to document a default value for a regular @param, eg

#' @param var This is a description. @default FALSE

to generate something like

Arguments: 

    `var`: This is a description. (Defaults to `FALSE`)
@danielinteractive
Copy link

My 2 cents on this default behavior: I would not use this. Because:

  1. default argument value as code can be read of the usage in the documentation already. So this would be duplicate.
  2. If any explanation of the default argument value is useful, then in human form, which cannot be parsed automatically. (Well at least as of now :-D)

@dgkf
Copy link
Collaborator Author

dgkf commented Feb 6, 2023

Yeah, I agree with you on both accounts, but I still see this pattern being used quite a lot. Even if the value is minor, I think there is some merit to it:

  1. It supports IDEs which use the argument definition as a tooltip for completions.
  2. It can help to clarify how defaults run through match.arg are used (which treats the formals as different from the default).
  3. It allows for documentation about default behaviors when the behavior is different from the value. You'll occasionally see functions that might have a default behavior equivalent to a value (e.g. FALSE) even if the default is NULL or missing.

Even if there's value here, I think it's clear that whatever value it might offer would be outside the needs of a documenting types. I'm in favor of shuffling this into another little pilot package and we can revisit it at a different time.

@danielinteractive
Copy link

danielinteractive commented Feb 6, 2023

Ah now I get it - yes as a manually specified tag this makes a lot of sense. Also fits in here. I would just not try to have the package infer the default automatically from the function definition, for above mentioned reasons.

I think for automatic population of anything in the roxygen chunk, might fit better in the https://github.com/yonicd/sinew package. Especially for the default value, it could be nice to prepopulate and then manually adjust (or delete).

@dgkf
Copy link
Collaborator Author

dgkf commented Feb 6, 2023

I would just not try to have the package infer the default automatically from the function definition, for above mentioned reasons.

Yep - that's disabled by default.

I think for automatic population of anything in the roxygen chunk..

The only reason I think it fits better as a roclet is that you can add checks for when defaults are undocumented. I think sinew already supports having a default as part of its template.

All things told, let's put this feature on hold then. Appreciate the feedback!

@danielinteractive
Copy link

Thanks Doug - I think the minimal version without automated generation is still very useful and would be nice to have

@dgkf
Copy link
Collaborator Author

dgkf commented Feb 13, 2023

Documenting decision:

I'm going to strip this feature out for now since I don't want to commit to supporting it without a bit of a better handle on how I want it to work or whether it should live alongside type definitions.

It will forever be part of the commit history, but for visibility I'm going to create a branch to house this experiment before stripping out the code.

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

No branches or pull requests

2 participants