amber-lang / amber

šŸ’Ž Amber the programming language compiled to Bash
https://amber-lang.com
GNU General Public License v3.0
3.81k stars 81 forks source link

[Feature] Make Amber header in generated Bash files configurable #344

Open Kami opened 1 month ago

Kami commented 1 month ago

Description

Today, header written by Amber compiler in the generated Bash files looks something like this:

#!/usr/bin/env bash
# Written in [Amber](https://amber-lang.com/)
# version: 0.3.4-alpha
# date: 2024-07-24 10:04:00

I think it's a good idea to, by default, include date when the file has been generated, but this is also problematic in some cases.

One of those cases is trying to determine if any changes have been made to the Amber source files and if the Bash files need to be re-generated.

Background, Context, Use Case

For reproducibility, transparency and other reasons, I check generated bash files into a git repository. I also have a CI check which runs "Verify Generated Bash Files Are Up To Date" step which verifies that all the generated files are up to date.

This of course doesn't work so well with dynamic header which is different on each compile run (I have some bash hacks to work around it and remove this problematic line, but it's not ideal).

Proposed Change / Implementation

I think in an ideal world, we would make the header fully end user configurable via CLI parameter and also support templates notation.

For example, something along the lines of:

amber --output-header "# Written in Amber\n version ${amber_version}\n date generated: ${date_generated}\n content sha512checksum: ${raw_content_sha512_checksum)" input.ab output.ab

Another (perhaps short term and simpler option) would be to add a new --output-header-skip-date flag which makes it skip inclusion of the script generation date / compiler run line in the output file.

Please let me know what other people think.

I'm happy to propose the actual change myself, but I would like to get some consensus on the approach first.

It's also worth noting that I'm not a Rust expert so implementing the more complex header option would take me longer than the second "exclude date CLI flag" approach.

Thanks.

Related PRs, Issues

Links

Mte90 commented 1 month ago

I think that you can go with a PR with the CLI parameter to not print it.

Consider also that Amber is not stable so we can change the Bash output as we are still in alpha so the issue you are facing it will be always there.

PS: the next alpha will have breaking changes as example

Ph0enixKM commented 1 month ago

Iā€™d just add (at least for now) --exclude-header flag and let users prepend their own headers. What do you think?

Kami commented 1 month ago

@Mte90

Thanks.

@Ph0enixKM

Iā€™d just add (at least for now) --exclude-header flag and let users prepend their own headers. What do you think?

That sounds good to me.

In fact, that's already what I do today.

In addition to the existing standard Amber header, I use sed to append a line similar along the lines of "DO NOT EDIT MANUALLY - This file has been generated by Amber compiler. Please see README.md on how to edit Amber source file and generate this file" after the shebang line.

That's also the reason why, long term, I think template approach may be the best. This way I could get rid of sed and replace it with native Amber functionality.

Having said that, I also prefer and appreciate KISS approach and if you think something like that doesn't belong in Amber core, I'm also fine with simple --exclude-header and writing header outside of Amber compiler.

Do you plan to make the change yourself? Otherwise I can do it (but it will take a while since I need to set up dev environment, etc and probably won't be able to do it before the weekend).

Thanks.

Mte90 commented 1 month ago

You can do a PR we are working on various stuff right now that have more priority. I think that as we are in alpha is fine a parameter and in the future create a template system.

b1ek commented 1 month ago

thats a rather non significant issue for now, but you can make a pr if you want tho.

id suggest making it kind of like fastfetch allows customizing outputs (just that module idea, no need to create a config file for that)

Ph0enixKM commented 1 month ago

Yeah let's keep it simple. The proposed flag should be sufficient for now.

It should be easy enough. If you need any help - let me know! You can reach me here under this Issue, on Discord or via Email šŸ™Œ