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: Syntax for controlling mixing of linter sources #1

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

Discussion: Syntax for controlling mixing of linter sources #1

dgkf opened this issue Feb 6, 2023 · 3 comments

Comments

@dgkf
Copy link
Collaborator

dgkf commented Feb 6, 2023

This is one of the more complex behaviors of the package. I'll start with background. There are a number of ways that linters can be added:

  1. roxylint provides its own default linters (that follow the tidyverse style guide for documentation).
  2. Other packages (eg roxytypes) can register linters as well. They'll be registered when they load or when roxytypes loads (if it wasn't already loaded)
  3. Users may define their own linters.

So, given a tag (let's say @param), there might be some default value, some additional linters provided by other packages, and some user-defined linters.

With this, a user should be able to specify whether they want to add to or overwrite existing linters, and assert that new linters should or shouldn't be allowed on top of the ones they've defined.

For now, providing a linter always overwrites existing linters and NULL is used as a sentinel value that says "do not allow more linters for this tag".

However, this isn't very pretty, is a bit unintuitive, and does not provide the ability to add to existing linters. Instead, we might imagine a special constructor:

list(
  linters = list(
    param = roxytypes::linters(mask = FALSE, lock = FALSE,
      function(x, name, description, ...) {
        n_caps <- nchar(gsub("[^[:upper:]]", "", description))
        if (n_caps < nchar(description) / 2)
          warning("descriptions should be at least 50% CaPiTALiZeD")
      }
    )
  )
)
@danielinteractive
Copy link

danielinteractive commented Feb 6, 2023

Yeah special constructor also works well I guess.
Maybe in general, I wonder how much you could lean on the lintr package for semantics and syntax to simplify learning curve for users.

@dgkf
Copy link
Collaborator Author

dgkf commented Feb 6, 2023

A couple thoughts inspired by the lintr comparison

  • tag-specific constructors might be nice so that you could do something like:
    list(linters = list(
      param = tidy_param(sentence_case = FALSE)
    ))
    This would be the equivalent to lintr's linters_with_defaults. These would be easy enough to add to just turn on-and-off specific default linters.
  • I think we'd still want to be able to "lock" or "mask" existing linters
  • The example above is an "advanced" usage. I already borrow from lintr's builtin defaults, so to mirror a more beginner-friendly usage that still uses the mask/lock wrapper, it could look something like:
    param = roxylint::linters(mask = FALSE, lock = FALSE,
      lint_sentence_case,
      lint_full_stop
    )
    I think this is about as straightforward as it could look since we still need to provide a linter style per-tag.
  • Not sure if this is what you had in mind, but I think the .R config file styled after roxygen2 is much more user friendly than lintr's .dcf file since you get all the conveniences of your IDE for writing R code.

@danielinteractive
Copy link

Thanks @dgkf yeah that looks nice! Yes .R is better.

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