Skip to content

Conversation

@AshesITR
Copy link
Collaborator

fixes #451

@AshesITR AshesITR changed the title Skip commented_code_linter if actuar parsed content was matched. Skip commented_code_linter if actual parsed content was matched. Jan 31, 2021
@AshesITR
Copy link
Collaborator Author

TIL parsed content includes COMMENT tokens :)

@MichaelChirico
Copy link
Collaborator

TIL parsed content includes COMMENT tokens

Why not drop the regex logic altogether? Extract text from COMMENT tokens and check parseability

@AshesITR
Copy link
Collaborator Author

I'm working on an XPath version, will push something later.

@AshesITR
Copy link
Collaborator Author

NB we still need the regex to extract potential code from the comments

all_comments <- xml2::xml_text(all_comment_nodes)
code_candidates <- re_matches(
all_comments,
rex(some_of("#"), any_spaces,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this part isn't needed anymore right? ditto for anything in the capture?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we need to anchor on the (first) # of the comment text, which will always be there in order to get the column_offset correct, but I'm not completely sure.

It depends on how the start and end locations of capture groups are returned by rex. If they are relative to the input string, we can probably prune the regex to only extract the code candidate to be analyzed. Note that we will still need to somehow anchor it to just after the comment start token, or it will match the entire string including the # which would always return a parseable (comment-only) code.

The COMMENT token will contain the # character starting the comment, so the refactoring as is is more or less sure to deliver the same candidate code as before if no # is present in actual code. WDYT?

Copy link
Collaborator

@MichaelChirico MichaelChirico Jan 31, 2021

Choose a reason for hiding this comment

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

Sure, agree it's safer not to try & rock the boat any more than necessary during a refactor.

@AshesITR AshesITR merged commit 148d283 into master Jan 31, 2021
@AshesITR AshesITR deleted the fix/451-commented-code branch January 31, 2021 22:28
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.

False positive for commented code

3 participants