Skip to content

Conversation

@Luni-4
Copy link
Collaborator

@Luni-4 Luni-4 commented Apr 7, 2020

  • Fix wrong implementation for CLOC
  • Add unit tests to verify the new implementation
  • Add CLOC explanation to the README

Thanks in advance for your review! :)

@Luni-4 Luni-4 requested a review from calixteman April 7, 2020 10:17
@Luni-4 Luni-4 force-pushed the fix-cloc branch 2 times, most recently from 958d1ae to 3718430 Compare April 7, 2020 16:43
String | DQUOTE | DQUOTE2 | ExpressionStatement | Block | Module => {}
Comment => {
stats.comment_lines.insert(start);
stats.comment_lines += (end - start) + 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

you probably need to handle the case of multiline comments:
https://twitter.com/gvanrossum/status/112670605505077248

Copy link
Collaborator Author

@Luni-4 Luni-4 Apr 10, 2020

Choose a reason for hiding this comment

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

I added a FIXME in the Python test because I don't know how to discriminate a string used as comment from a simple string

Copy link
Collaborator

Choose a reason for hiding this comment

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

An idea could be to look for string where the parent is an ExpressionStatement. I'd say that in the case the string is the statement and so this the only thing we've so a comment. Wdyt ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, good idea! I'm going to implement that, thanks @calixteman! :)

@calixteman calixteman merged commit 9b527ea into mozilla:master Apr 10, 2020
@Luni-4 Luni-4 deleted the fix-cloc branch April 10, 2020 11:18
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.

2 participants