Added more robust comment format support. #82

allenh1 · 2017-08-04T18:58:52Z

I've tested this as-is with the following:

/*
 * slam_gmapping
 * Copyright (c) 2008, Willow Garage, Inc.
 * Copyright (c) 2017, Open Source Robotics Foundation, Inc.
 *
 * THE WORK (AS DEFINED BELOW) IS PROVIDED UNDER THE TERMS OF THIS CREATIVE
 * COMMONS PUBLIC LICENSE ("CCPL" OR "LICENSE"). THE WORK IS PROTECTED BY
 * COPYRIGHT AND/OR OTHER APPLICABLE LAW. ANY USE OF THE WORK OTHER THAN AS
 * AUTHORIZED UNDER THIS LICENSE OR COPYRIGHT LAW IS PROHIBITED.
 *
 * BY EXERCISING ANY RIGHTS TO THE WORK PROVIDED HERE, YOU ACCEPT AND AGREE TO
 * BE BOUND BY THE TERMS OF THIS LICENSE. THE LICENSOR GRANTS YOU THE RIGHTS
 * CONTAINED HERE IN CONSIDERATION OF YOUR ACCEPTANCE OF SUCH TERMS AND
 * CONDITIONS.
 *
 */

and got

src/slam_gmapping.cpp: copyright=Willow Garage, Inc. (2008), Open Source Robotics Foundation, Inc. (2017), license=<unknown>

which seems to be correct (since we don't have a BSD template).

I do not know enough to call this resolved, however.

connects to #75

This change is

allenh1 · 2017-08-04T19:00:10Z

ament_copyright/ament_copyright/parser.py

@@ -175,7 +175,7 @@ def search_copyright_information(content):
    year = '\d{4}'
    year_range = '%s-%s' % (year, year)
    year_or_year_range = '(?:%s|%s)' % (year, year_range)
-    pattern = '^[^\n\r]?\s*Copyright(?:\s+\(c\))?\s+(%s(?:,\s*%s)*),?\s+([^\n\r]+)$' % \
+    pattern = '^[^\n\r]?[\s\*\w]*Copyright(?:\s+\(c\))?\s+(%s(?:,\s*%s)*),?\s+([^\n\r]+)$' % \


I'm very hesitant to allow for arbitrary alpha-numeric characters here... I don't know enough about licensing to state if this is valid.

Why is this necessary? Is this aiming to allow newlines anywhere in the comment? Would it be sufficient to only make it flexible within the license?

This is because of things like

/* * My Amazing Project * Copyright (c) 2017

I saw a lot of that in some old willow code (like GMapping).

I don't think the \* is needed, however.

allenh1 · 2017-08-04T19:00:43Z

ament_copyright/ament_copyright/parser.py

@@ -248,6 +249,8 @@ def get_comment_block(content, index):
    comment_token = match.group(1)
    start_index = match.start(1)

+    if comment_token in ['/*', '/**']:
+        comment_token = ' *'


This is rather unfortunate -- does anyone see a better way?

I think the leading space should not be required. This should be valid:

/** * text */

Ok -- I can add that functionality.

/** text */

is that also acceptable?

I'm thinking something like this:

if comment_token in ['/*', '/**']: comment_token = '*' """ more code """ lines = [line[line.index(comment_token) + 1:] for line in lines]

That works for the existing copyright cases, but not for the new ones.

allenh1 · 2017-08-09T19:57:58Z

I've determined I don't have the time to make this as robust as I'd like, as it will require a very significant rewrite in the parser.py file (many assumptions are made based on the fact that the comments start with the same prefix on every line, and that they are on every line).

This PR only supports

/*
 *
 */

because of the * on the end of the last comment line. This does, however, add the desired newline robustness.

mikaelarguedas · 2017-08-10T21:55:04Z

@allenh1 if you are not working on this anymore, can you reflect it in the labels? (by removing the 'in progress' label from this and the parent issue)

allenh1 · 2017-08-10T22:00:51Z

@mikaelarguedas done

mikaelarguedas · 2017-08-30T21:29:49Z

@wjwwood FYI:
This works for bloc comments right now.

/*
 * stuff
 */

Current cases not covered:

more than 1 space between comment marker and text:

/*
 *      stuff
 */

no space between comment marker and text:

/*
 *stuff
 */

not leading * for each line within the block comment

/*
stuff
*/

not leading space before *

/*
* stuff
*/

greimela-si · 2018-07-11T12:52:57Z

@wjwwood @mikaelarguedas Are there any plans to merge this?

wjwwood · 2018-07-11T21:07:58Z

It's not on my todo-list right now. I doubt I'll have time to address it in the short-term.

Working around ament/ament_lint#82

Working around ament/ament_lint#82 Signed-off-by: Tully Foote <tfoote@osrfoundation.org>

dirk-thomas · 2019-07-30T15:47:46Z

Closing due to no activity.

Recent related work: #160.

Added more robust comment format support.

ccc41fe

allenh1 added the in progress Actively being worked on (Kanban column) label Aug 4, 2017

allenh1 self-assigned this Aug 4, 2017

allenh1 commented Aug 4, 2017

View reviewed changes

Remove extra '\*' in the copyright check regex.

53a6e3d

allenh1 mentioned this pull request Aug 9, 2017

Make ament_copyright robust to newlines #75

Open

allenh1 removed the in progress Actively being worked on (Kanban column) label Aug 10, 2017

mikaelarguedas mentioned this pull request Aug 29, 2017

OpenSplice update ros2/rmw_opensplice#187

Closed

mikaelarguedas assigned wjwwood and unassigned allenh1 Aug 30, 2017

wjwwood mentioned this pull request Aug 31, 2017

Linting for rviz_rendering ros2/rviz#17

Merged

dirk-thomas added the enhancement New feature or request label Feb 22, 2018

wjwwood added the help wanted Extra attention is needed label Jul 11, 2018

wjwwood removed their assignment Jul 11, 2018

greimela-si mentioned this pull request Jul 24, 2018

First working rosbag2 implementation ros2/rosbag2#6

Merged

This was referenced Aug 2, 2018

Rebased joystick_drivers on the latest ros2/joystick_drivers#8

Merged

This is an import of assert.h from rosconsole ros2/rcutils#112

Closed

tfoote added a commit to ros2/rcutils that referenced this pull request Aug 6, 2018

changing license comment style

a50db4d

Working around ament/ament_lint#82

tfoote added a commit to ros2/rcutils that referenced this pull request Aug 7, 2018

changing license comment style

828b267

Working around ament/ament_lint#82

tfoote added a commit to ros2/rcutils that referenced this pull request Jul 24, 2019

changing license comment style

860758f

Working around ament/ament_lint#82

tfoote added a commit to ros2/rcutils that referenced this pull request Jul 24, 2019

changing license comment style

02d2654

Working around ament/ament_lint#82 Signed-off-by: Tully Foote <tfoote@osrfoundation.org>

dirk-thomas closed this Jul 30, 2019

dirk-thomas deleted the copyright-newline-robustness-75 branch July 30, 2019 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added more robust comment format support. #82

Added more robust comment format support. #82

allenh1 commented Aug 4, 2017 •

edited

Loading

allenh1 Aug 4, 2017

dirk-thomas Aug 4, 2017

allenh1 Aug 4, 2017

allenh1 Aug 4, 2017

allenh1 Aug 4, 2017

dirk-thomas Aug 4, 2017

allenh1 Aug 4, 2017

allenh1 Aug 4, 2017

allenh1 commented Aug 9, 2017

mikaelarguedas commented Aug 10, 2017

allenh1 commented Aug 10, 2017

mikaelarguedas commented Aug 30, 2017

greimela-si commented Jul 11, 2018

wjwwood commented Jul 11, 2018

dirk-thomas commented Jul 30, 2019

Added more robust comment format support. #82

Added more robust comment format support. #82

Conversation

allenh1 commented Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allenh1 commented Aug 9, 2017

mikaelarguedas commented Aug 10, 2017

allenh1 commented Aug 10, 2017

mikaelarguedas commented Aug 30, 2017

greimela-si commented Jul 11, 2018

wjwwood commented Jul 11, 2018

dirk-thomas commented Jul 30, 2019

allenh1 commented Aug 4, 2017 •

edited

Loading