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

Unify the way optional params are displayed. #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abadc0de
Copy link

LDoc has two different ways to annotate optional parameters, [opt] and [type=?foo]. This patch attempts to resolve the differences in the way optional parameters display: [opt] params are shown as "optional" in the param description, and [type=?] params are shown in brackets in the param list. Also added a --plain_params setting to turn off brackets and default values in the param list.

  • Params with modifiers like [type=?foo] have brackets in param list.
  • Params with modifiers like [opt] show up as "optional" in param description.
  • Add -P,--plain_params setting for no brackets or defaults in param list.

- Add -P,--plain_params setting for no brackets or defaults in param list.
- Params with modifiers like [type=?foo] have brackets in param list.
- Params with modifiers like [opt] show up as "optional" in param description.
@abadc0de
Copy link
Author

@stevedonovan, note that mixed type modifiers like [type=?foo|bar] don't show up as optional in the description. I wasn't sure if this was a bug or intentional. Since they will show up in brackets in the param list with this patch, they should probably also show up as optional in the description, for uniformity. I wanted to check with you first before making that change, though.

@abadc0de
Copy link
Author

I'm having second thoughts about this part:

  • Params with modifiers like [type=?foo] have brackets in param list.

If those params are only followed by optional params or are at the end of the list, it's fine. But what about a function documented like this:

send_head(status_code[, status_reason], headers)

This implies that if you don't want to pass a "status_reason," you can call it like this (and the function will juggle the arguments):

send_head(status_code, headers)

If that's the case you could document it like this:

--- Send the first part of an HTTP response.
-- @param[type=number] status_code
-- @param[opt,type=string] status_reason
-- @param[type=table] headers
send_head(status_code, status_reason, headers)

But what if the function does not do argument-juggling and it needs to be called like this:

send_head(status_code, nil, headers)

In this case it shouldn't have brackets, because the brackets imply you can leave that argument out of the list entirely. Without this patch you could document it like this:

--- Send the first part of an HTTP response.
-- @param[type=number] status_code
-- @param[type=?string] status_reason
-- @param[type=table] headers
send_head(status_code, status_reason, headers)

However there is no way to document an untyped, "non-juggled" parameter, e.g. [type=?], and no way to assign it a default value.

I'm thinking about making the following changes:

  • Don't use brackets for any [type=?foo] params if they're not chained with the end of the list.
    • This applies even to params with [opt]; this way [opt=...] can be used to annotate default values for "non-juggled" params. This changes the current meaning of [opt] when combined with [type=?foo]. OR
    • Params with [opt] still get brackets; add a param modifier [default=...] to annotate default values for "non-juggled" params. This retains the current meaning of [opt], but introduces a new pmod.
  • Allow modifiers like [type=?], with no type name following the question mark, to handle the case described above (where you want to mark something as optional, but it needs a placeholder nil in the arguments list, and you don't want to specify a type).

Thoughts?

@alerque
Copy link
Member

alerque commented Sep 29, 2020

Seven years is enough room for a lot of second thoughts. I've just picked up helping to maintain this and I'm not able to make out if this is still relevant or not. Maybe you can clue me in?

@abadc0de
Copy link
Author

Wow, blast from the past. I believe this was just an attempt to make optional arguments and nullable arguments end up meaning the same thing internally (that is, @param[opt,type=string] blah and @param[type=?string] blah are interpreted the same way). No idea if it's still relevant to modern LDoc, sorry!

@alerque
Copy link
Member

alerque commented Sep 29, 2020

You past just landed on my present :-/

Okay I'll take a look. That conflation came up in a couple other PRs I was reviewing too: nil, false, and 'none' were all being handled by the same variable. Ugg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants