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

refactor xml_nodes_to_lints() lint location to XPath #1234

Merged
merged 14 commits into from
May 25, 2022

Conversation

AshesITR
Copy link
Collaborator

@AshesITR AshesITR commented May 25, 2022

Fixes #1215

  • move xml_nodes_to_lints() to its own file
  • always return unnamed line info in Lint()s
  • allow passing a list() containing xml_nodes, even if it's not classed xml_nodeset
  • refactor match_after_end usages to XPath based locations
  • use new XPath based locations to improve location information for commas_linter()
  • update tests for commas_linter()
  • add tests for xml_nodes_to_lints()
  • refactor spaces_inside_linter() to use xml_nodes_to_lints()

R/commas_linter.R Outdated Show resolved Hide resolved
#' to the `Lint` object. If `lint_message` is a `function`,
#' this function is first applied to `xml` (so it should be a
#' function taking an `xml_node` as input and must produce a
#' length-1 character as output). If `lint_message` is a character vector the same length as `xml`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to add a test to enforce the equal-lengths part more strictly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonequal lengths will generate a recycling warning by .mapply(), right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right... tidyverse usually hates recycling though and tends to error instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good candidate for a sanity-check follow up (in addition to the start <= column <= nchar(line) check discussed below.

@MichaelChirico
Copy link
Collaborator

last few comments here. good overall! I think this API maximizes flexibility & has good defaults.

@AshesITR
Copy link
Collaborator Author

The build failure looks sporadic. Restarting should fix it.

@AshesITR AshesITR merged commit 3bc7cd5 into main May 25, 2022
@AshesITR AshesITR deleted the feature/xml_nodes_to_lints-location_xpaths branch May 25, 2022 21:07
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

Successfully merging this pull request may close these issues.

improve source markers for commas_linter
2 participants