Skip to content
This repository has been archived by the owner on Jul 18, 2022. It is now read-only.

Improve similar header matching #3

Open
fbiville opened this issue Sep 26, 2018 · 4 comments
Open

Improve similar header matching #3

fbiville opened this issue Sep 26, 2018 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@fbiville
Copy link
Owner

fbiville commented Sep 26, 2018

That would help users to perform the initial migration.

The idea could be: match the first comment section before the package declaration, if the header to insert is similar enough (for some value of enough), then let's replace it.

@fbiville
Copy link
Owner Author

fbiville commented Oct 15, 2018

For the immediate client aka projectriff, matching extra commented newlines is enough (and is now done).

@fbiville fbiville added the enhancement New feature or request label Oct 16, 2018
@fbiville
Copy link
Owner Author

fbiville commented Oct 26, 2018

Made the comparison case-insensitive, ignore starting whitespace chars as well as trailing ones and dots

@fbiville fbiville changed the title Match header by similarity Improve similar header matching Jan 29, 2019
@fbiville
Copy link
Owner Author

fbiville commented Apr 4, 2019

One example where headache is too sensitive to variations (on the first run): http URL instead of https (real-life example).
Let's say your license header is as follows:

My awesome license - Copyright {{.YearRange}} {{.Author}}

See more: https://mylicense.example.com

And some files have headers similar but with http URLs:

// My awesome license - Copyright 2012-2013 The original author or authors
// 
// See more: http://mylicense.example.com
[...]

Running headache on this file would cause the old header not to be detected and the new header to be appended before the old one.

To solve this issue, here is a trick one can apply. Parameterize the protocol:

My awesome license - Copyright {{.YearRange}} {{.Author}}

See more: {{.Protocol}}://mylicense.example.com

Add the corresponding definition in the JSON configuration:

[...]
  "data": {
    [...]
    "Protocol": "https"
  }

Run headache and you'll see it matches http and replaces it with https!
Why? Because headache is able to match anything where you define placeholders :)

Once everything is normalized, you can remove the parameter, come back to the initial header license template and run headache again just to make sure it works fine.

You do not need to resort to this if headache has run previously and you want to switch from http to https. Changing the template and re-running headache will be sufficient, as headache is able to parse the last used configuration and match headers accordingly. The above piece of advice applies only for first-time execution.

@fbiville fbiville added the help wanted Extra attention is needed label Apr 4, 2019
@fbiville
Copy link
Owner Author

fbiville commented Apr 4, 2019

Current shortcoming: if the new license header contains an extra newline somewhere, that causes similar headers to not be matched at all. Supporting this should not require to change the whole approach (see #50).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant