pre-commit-hooks icon indicating copy to clipboard operation
pre-commit-hooks copied to clipboard

insert_license - Only first year range is replaced in existing license

Open pezcode opened this issue 3 years ago • 6 comments

When a license exists in a file and insert_license with --use-current-year updates the year, it only does so for the first line containing a year:

 * Copyright © 2020-2022 Steve Luamo <[email protected]>
 * Copyright © 2015-2022 Leve Stuamo <[email protected]>

becomes (note the 2022 in the second line):

 * Copyright © 2020-2023 Steve Luamo <[email protected]>
 * Copyright © 2015-2022 Leve Stuamo <[email protected]>

This seems to be intentional since the comment in try_update_year_range() says "The change will affect only the first line containing years". But I'm not sure why this limitation exists and whether it could be lifted.

pezcode avatar Jan 26 '23 18:01 pezcode

Ping @aostrowski who implemented this 😊

As for myself, I'd be open to perform the changes on all matching lines in the header

Lucas-C avatar Jan 26 '23 21:01 Lucas-C

I put this in place because I assumed if multiple people are present in the header, only the one who made changes should have the year updated.

I tried to find real-world examples of files with multiple numbers in license headers. In most cases, I guess the correct behavior would be to update just the last spotted year, for instance like in those cases: https://github.com/torvalds/linux/blob/master/include/misc/altera.h, https://github.com/torvalds/linux/blob/master/include/video/sa1100fb.h.

In the first case, other companies were developing the header earlier on, so their copyrights shouldn't be extended when other developers build on their work. In the second case, an LCD controller have 4 digits in the name, which would be treated as a year instead of the actual copyright year.

I can modify the code to treat all numbers as years, or I can treat just the last one. A third option would be to just update the year on lines containing the word "Copyright". Perhaps this would be the best option.

Which approach do you think would work in most cases?

aostrowski avatar Jan 28 '23 14:01 aostrowski

Thanks for the fast reply ☺️

I think finding an approach that works for everything is hard. In our case we do need support for updating multiple ranges. There are two options I can think of:

  • add another argument to select whether to update the first/last/all years (or ranges)
  • find a way to annotate years in the input license file that need updating, would also fix the LCD controller issue. Either some placeholder for "insert current year" or a way to mark "this is a year" - if neither is found revert to current behavior.

I understand if these are too intrusive for your taste, and adding the changes in our own fork wouldn't be terribly hard.

pezcode avatar Jan 29 '23 23:01 pezcode

Thank you both for your thoughts on the subject.

I think --use-current-year --update-all-year-ranges could be a simple solution that should please everyone.

Are you OK with that solution? Is one of you interested in contributing a PR for this?

Lucas-C avatar Feb 01 '23 07:02 Lucas-C

--update-all-year-ranges would work well 😌 The behavior should then match for both existing and updated licenses. Currently newly inserted licenses have all years/ranges updated, but should only update the first unless --update-all-year-ranges is given.

I can take a stab at a PR for this, might just take a few days before I can find the time.

pezcode avatar Feb 01 '23 11:02 pezcode

I can take a stab at a PR for this, might just take a few days before I can find the time.

Great! Sure, take your time 😊

Lucas-C avatar Feb 01 '23 12:02 Lucas-C