Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[unstable option] license_template_path #3352

Closed
scampi opened this issue Feb 13, 2019 · 14 comments
Closed

[unstable option] license_template_path #3352

scampi opened this issue Feb 13, 2019 · 14 comments
Labels
unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for unstable option: license_template_path

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@scampi scampi changed the title [unstable option] unstable option: license_template_path [unstable option] license_template_path Feb 18, 2019
@scampi
Copy link
Contributor Author

scampi commented Apr 8, 2019

@Robbepop
Copy link

Hi, I am using this feature and it helps me a lot in general with the license headers in my project.
So far I used something like this to specify the valid dates in the license:

{20\d{2}}-{20\d{2}}

This works and successfully validates date ranges such as 2018-2019.
However, I also wanted to make the last part optional and thus only allow a single year to be valid as well.
I tried several things, none worked.

{20\d{2}}(?:-{20\d{2}})? // nope
{20\d{2}}(?:-{20\d{2}}?) // nope
{20\d{2}}{{20\d{2}}}? // nope

Maybe I am doing something wrong. (I am not used to work with regex.)
Or are there some limits to the regex parser for license headers?
If so we should either document those or maybe tackle them. :)

@thibault-martinez
Copy link

Hi @Robbepop, if still relevant, this works for me {20\d{2}(-20\d{2})?}.

@Robbepop
Copy link

Robbepop commented May 4, 2020

Thanks I will try that out! :)

@mrbianchi
Copy link

mrbianchi commented Aug 26, 2020

Multiline template isn't working...
error: license check failed

@mrbianchi
Copy link

mrbianchi commented Aug 26, 2020

Looks like my issue is using Windows and the break lines, I cloned repositories with that and still failing, I'm using the last nightly version

@mrbianchi
Copy link

Switched CRLF to LF in the template file and now is working, I was using visual studio code
https://stackoverflow.com/a/39532890

@MantisClone
Copy link

I was confused about what regex formatting options were allowed. The comment at license.rs#L70 proved useful.

@calebcartwright
Copy link
Member

Thanks for sharing @mrbianchi! line endings are a bit of a pain for rustfmt to work with as the rustc parser normalizes everything to unix style endings in the source map that rustfmt works with, and we then go back in towards the latter formatting phases (after the license template check is performed) to perform line ending checks and, if necessary, the converted value.

It could be possible with a refactor to shift that check after the line ending conversions, if you or anyone else is interested in investigating. In the meantime, I imagine it'd be helpful if the documentation for the license_template_path configuration option explicitly called out the need to use unix style line endings in the template file? That can be updated here for anyone interested.

rustfmt/src/formatting.rs

Lines 206 to 227 in 30bda45

format_lines(
&mut visitor.buffer,
&path,
&visitor.skipped_range.borrow(),
config,
report.clone(),
);
// SourceFile's in the SourceMap will always have Unix-style line endings
// See: https://github.com/rust-lang/rustfmt/issues/3850
// So we must check the file system to get the original file value in order
// to detect newline_style conflicts.
// Otherwise, parse session is around (cfg(not(test))) and newline_style has been
// left as the default value, then try getting source from the parse session
// source map instead of hitting the file system.
let original_text = match original_snippet {
Some(snippet) => snippet,
None => std::fs::read_to_string(path.as_path().ok_or_else(|| {
OperationError::IoError(std::io::Error::from(std::io::ErrorKind::InvalidInput))
})?)?,
};
apply_newline_style(config.newline_style(), &mut visitor.buffer, &original_text);

rustfmt/src/formatting.rs

Lines 308 to 328 in 30bda45

fn format_lines(
text: &mut String,
name: &FileName,
skipped_range: &[NonFormattedRange],
config: &Config,
report: FormatReport,
) {
let mut formatter = FormatLines::new(name, skipped_range, config);
if let Some(false) = formatter.check_license(text) {
report.add_license_failure(name.clone());
}
formatter.iterate(text);
if formatter.newline_count > 1 {
debug!("track truncate: {} {}", text.len(), formatter.newline_count);
let line = text.len() - formatter.newline_count + 1;
text.truncate(line);
}
report.append_errors(name.clone(), formatter.errors.into_iter());
}

@calebcartwright
Copy link
Member

@DMATS - Thanks for sharing! Do you think the wording of the description/help text for the config option could be phrased differently to better express this?

@cbarrick
Copy link

The template path appears to be resolved relative to the current directory when running cargo fmt.

This causes an error when running the command from a sub-directory in a workspace:

Warning for license template file "header.rs": No such file or directory (os error 2)

I was not expecting cargo fmt to depend on me being in the workspace root directory.

My expectation:

  • When specified in rustfmt.toml, resolve the template path relative to that file.
  • When specified with --config, resolve the template path relative to the current directory.

@gilescope
Copy link

gilescope commented Jul 30, 2021

Hmm, got a few different licenses in one workspace.
I basically wanted to ensure that each file has got a license set:
{.*}SPDX-License-Identifier: {.*}

Seems like the regex's only span one line. Ideally I don't mind which line they put this in their header as long as they've got one. (Alternatively if license_template_path could take a list of templates and passed if it matched any of them that would work.)

It seems I can copy my .rustfmt into a subdir and remove a license check (is there a way I can avoid duplicating all the other settings? Ideally I'd like to use all the previous settings and then drop the check: license_template_path = "" ).

@gilescope
Copy link

@cbarrick agree with you about the relative dir. Here's an attempt at fixing that: rust-lang/rust#87627

@calebcartwright
Copy link
Member

Going to close this because stabilization doesn't make sense given our intent to deprecate and remove the option, refs #5103

mattschlebusch added a commit to mattschlebusch/firecracker that referenced this issue Aug 19, 2022
Adds string handling back into pre-commit to drop quotes for
rustfmt --config option to be able to use the string values
as unquoted.

This change additionally drops the license-template-path
formatting parameter as it is no longer supported by the
rust toolchain. See github discussion:
rust-lang/rustfmt#3352

Signed-off-by: Matthew Schlebusch <[email protected]>
mattschlebusch added a commit to mattschlebusch/firecracker that referenced this issue Aug 22, 2022
Drops string handling from pre-commit entirely.
rustfmt --config option can't be used with the parameters
'as is' (string values as unquoted). Instead use
rustfmt --config-path that takes the whole fmt.toml file.

To be able to use --config-path requires the nightly toolchain
to be added to the docker dev container as well.

This change additionally drops the license-template-path
formatting parameter as it is no longer supported by the
rust toolchain. See github discussion:
rust-lang/rustfmt#3352

Signed-off-by: Matthew Schlebusch <[email protected]>
mattschlebusch added a commit to mattschlebusch/firecracker that referenced this issue Aug 22, 2022
Drops string handling from pre-commit entirely.
rustfmt --config option can't be used with the parameters
'as is' (string values as unquoted). Instead use
rustfmt --config-path that takes the whole fmt.toml file.

To be able to use --config-path requires the nightly toolchain
to be added to the docker dev container as well.

This change additionally drops the license-template-path
formatting parameter as it is no longer supported by the
rust toolchain. See github discussion:
rust-lang/rustfmt#3352

Signed-off-by: Matthew Schlebusch <[email protected]>
mattschlebusch added a commit to mattschlebusch/firecracker that referenced this issue Aug 22, 2022
Drops string handling from pre-commit entirely.
rustfmt --config option can't be used with the parameters
'as is' (string values as unquoted). Instead use
rustfmt --config-path that takes the whole fmt.toml file.

To be able to use --config-path requires the nightly toolchain
to be added to the docker dev container as well.

This change additionally drops the license-template-path
formatting parameter as it is no longer supported by the
rust toolchain. See github discussion:
rust-lang/rustfmt#3352

Signed-off-by: Matthew Schlebusch <[email protected]>
mattschlebusch added a commit to mattschlebusch/firecracker that referenced this issue Aug 22, 2022
Drops string handling from pre-commit entirely.
rustfmt --config option can't be used with the parameters
'as is' (string values as unquoted). Instead use
rustfmt --config-path that takes the whole fmt.toml file.

To be able to use --config-path requires the nightly toolchain
to be added to the docker dev container as well.

This change additionally drops the license-template-path
formatting parameter as it is no longer supported by the
rust toolchain. See github discussion:
rust-lang/rustfmt#3352

Signed-off-by: Matthew Schlebusch <[email protected]>
mattschlebusch added a commit to mattschlebusch/firecracker that referenced this issue Aug 22, 2022
Drops string handling from pre-commit entirely.
rustfmt --config option can't be used with the parameters
'as is' (string values as unquoted). Instead use
rustfmt --config-path that takes the whole fmt.toml file.

To be able to use --config-path requires the nightly toolchain
to be added to the docker dev container as well.

This change additionally drops the license-template-path
formatting parameter as it is no longer supported by the
rust toolchain. See github discussion:
rust-lang/rustfmt#3352

Signed-off-by: Matthew Schlebusch <[email protected]>
mattschlebusch added a commit to mattschlebusch/firecracker that referenced this issue Aug 31, 2022
Drops string handling from pre-commit entirely.
rustfmt --config option can't be used with the parameters
'as is' (string values as unquoted). Instead use
rustfmt --config-path that takes the whole fmt.toml file.

To be able to use --config-path requires the nightly toolchain
to be added to the docker dev container as well.

This change additionally drops the license-template-path
formatting parameter as it is no longer supported by the
rust toolchain. See github discussion:
rust-lang/rustfmt#3352

Signed-off-by: Matthew Schlebusch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unstable option tracking issue of an unstable option
Projects
None yet
Development

No branches or pull requests

8 participants