Implemented AMBER_HEADER and AMBER_FOOTER variables #682
Conversation
|
i've edited this a bit to be more consistent with the rest of our codebase and i think this is ready to be reviewed & merged. i also hardcoded newline between header and footer to prevent the first line being commented out if the user supplies their own header which doesn't have a newline at the end, like this: long text with code# my custom header
# amber version: 1.4.0echo hi! |
b1ek
requested review from
hdwalters and
Ph0enixKM
and removed request for
hdwalters
There was a problem hiding this comment.
Do we want to keep the date? Since the produced output can have a different date based on the same input, this breaks the reproducible build principle. Perhaps should we let the OS store the creation date in the file's metadata.
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/env bash | |||
| # Written in [Amber](https://amber-lang.com/) | |||
| # version: {{ version }} | |||
| # date: {{ date }} | |||
| # date: {{ date }} | |||
There was a problem hiding this comment.
Consider adding newline in the end of the file
| # date: {{ date }} | |
| # date: {{ date }} | |
There was a problem hiding this comment.
i've outlined before why it has to be hardcoded, so adding a newline here would mean that there are two newlines in the output
There was a problem hiding this comment.
Ah yes. We could header.trim_end() so that this file can be saved properly. Developers using custom headers would have to remember not to put the additional line too.
|
|
||
| let header_template = | ||
| if let Ok(dynamic) = env::var("AMBER_HEADER") { | ||
| fs::read_to_string(dynamic.to_string()).expect(format!("Couldn't read the dynamic header file from {dynamic}").as_str()) | ||
| } else { | ||
| include_str!("header.sh").to_string() | ||
| }; | ||
|
|
||
| let footer_template = | ||
| if let Ok(dynamic) = env::var("AMBER_FOOTER") { | ||
| fs::read_to_string(dynamic.to_string()).expect(format!("Couldn't read the dynamic footer file from {dynamic}").as_str()) | ||
| } else { | ||
| String::new() | ||
| }; | ||
|
|
||
| let header = header_template | ||
| .replace("{{ version }}", env!("CARGO_PKG_VERSION")) | ||
| .replace("{{ date }}", now.as_str()); | ||
|
|
||
| let footer = footer_template |
There was a problem hiding this comment.
This section is pretty long and adds up to the length of the translate method. Would you consider extracting it to a new private gen_header (or something similar) method?
This fixes #344 and #672 by introducing an
AMBER_HEADERvariable that points to the customizable header file, as well as anAMBER_FOOTERvariable to optionally append raw data to the end of the script.Again, this is a proof-of-concept, and it stands to reason that it could be implemented in a better way, but this can serve as a starting point.