amber-lang / amber

💎 Amber the programming language compiled to bash
https://amber-lang.com
GNU General Public License v3.0
3.77k stars 80 forks source link

⚙️ __AS=$? on every command also if the status isn't checked #183

Open Mte90 opened 2 months ago

Mte90 commented 2 months ago

So the amber script is:

silent unsafe {
    $cd lua-language-server$
    $git pull$
    $./make.sh$
    $ln -s /opt/lua-language-server/bin/lua-language-server /usr/local/bin/lua-language-server$
}

Generate this bash script:

cd lua-language-server >/dev/null 2>&1
__AS=$?
git pull >/dev/null 2>&1
__AS=$?
./make.sh >/dev/null 2>&1
__AS=$?
ln -s /opt/lua-language-server/bin/lua-language-server /usr/local/bin/lua-language-server >/dev/null 2>&1
__AS=$?
cd /tmp >/dev/null 2>&1
__AS=$?

Create that variable also if not used is not helpful at all in the bash script generated. I think that if unsafe it is used that variable shouldn't be added.

Mte90 commented 2 months ago

I think that should be enough to check on https://github.com/Ph0enixKM/Amber/blob/69e96ae3867091bcd374ff3cf42909324e73646e/src/modules/expression/literal/status.rs#L28 if the command is unsafe to not print that

Ph0enixKM commented 2 months ago

But if developer uses unsafe modifier does this imply that they don't want to check the status later on? @Mte90

I think this should be a part of rendered code optimisation for Amber

Mte90 commented 2 months ago

If the status is not used after why add code anyway? This is kind of a compiler optimization to generate only useful code.

Mte90 commented 1 month ago

So checking my experimental script to evaluate improvements to Amber https://github.com/Mte90/My-Scripts/blob/master/dev/lsp-installer/install.sh

We can see as today tons of __AS=$? that are useless.

Mte90 commented 1 month ago

So https://github.com/amber-lang/amber/blob/master/src/modules/condition/failed.rs#L95 adds the useless variable. I think that this code should check if the next line in the amber code is checking for the status variable.

Mte90 commented 1 month ago

Maybe the approach should be the apposite looking at the code.

We add __AS=$? basically at every command, instead when it is called status we should add it the line before.

We can't do it on https://github.com/amber-lang/amber/blob/master/src/modules/expression/literal/status.rs#L30 as it is just a replace so how we can do that (I am saying just to understand how works)?

Ph0enixKM commented 1 month ago

The solution that I came up with is recorded on this video: https://www.youtube.com/watch?v=_caXIMxrYj8 We could implement this optimization for this issue