Open b1ek opened 1 month ago
Really good job implementing this @b1ek! Here is some bug that I found.
When I ran this code that used a standard library function split
it failed with this error:
It seems that it uses a predefined variable to set the IFS (Internal Field Separator) for just this command.
Can we detect if a command is setting some variable before running something else? If so then can we find the real command? If not then we could omit or emit some kind of warning perhaps that the RDC is disabled for this command.
The bad part is that the variable assignment could be like this: IRC=" "
. I think we have to parse the bash commands to some extent. Also there is this edge case where user could do this:
let command = "echo"
${command} "Hello World"$
@boushley @arapower can you take a look at this?
Can we detect if a command is setting some variable before running something else?
Yeah, looks pretty simple
Also there is this edge case where user could do this:
let command = "echo" ${command} "Hello World"$
im not sure that a thing like that can be caught compile time, without rethinking the type system. perhaps we could create a type that would only allow certain strings, such as in typescript:
let cmd: 'echo' | 'curl';
and then check for all those strings in RDC.
but all of that seems as too much work for too little gain. my opinion is to let the user know that if they are specifying a command dynamically, RDC won't catch it. a workaround is possible:
silent unsafe $some_cmd --help$ // this is a no-op, but RDC will add this to its list
let command = "some_cmd"
${some_cmd} --stuff$
a workaround is possible:
silent unsafe $some_cmd --help$ // this is a no-op, but RDC will add this to its list let command = "some_cmd" ${some_cmd} --stuff$
actually, it would be pretty neat to add a compiler flag to tell RDC what other commands should it check for, without impacting the output file
Yeah, looks pretty simple
This doesn't seem to match the following string though:
IFS="hello world" read --arg1 --arg2
This doesn't seem to match the following string though:
IFS="hello world" read --arg1 --arg2
you're right.
this one should match any valid bash syntax: ^ *\S+=((\$|)\(.*\)|("|').*("|')|(\S+\\ |)+\S+|) *( *\S+=((\$|)\(.*\)|("|').*("|')|(\S+\\ |)+\S+|)|) *
.
i love writing regex.
i feel like this should be maintained as a separate project, so it would work with any bash file, with a possibility of integrating it into amber later
We should continue this discussion on Github Discussions that I'll create this evening. This PR is great and I think we can leave the basic RDC for Amber itself.
I'd remove the REGEX parsing of commands to search for the dependencies or I'd move them to a separate project. But I think that we need to discuss this whole idea first.
By the way, wouldn't it be better to merge the following commit as a separate PR?
By the way, wouldn't it be better to merge the following commit as a separate PR?
* [2ff9022](https://github.com/Ph0enixKM/Amber/commit/2ff902212321f912614663600e4ad8ae542e152c)
yeah, i agree with that. opening a new PR now
turns out implementing this wasn't as complicated as i thought.
features added:
cargo test
s--disable-rdc
possible caveats:
regex
crate is added as well. It may increase the compiler binary size, but shouldn't be much.AMBER_RDC_RD
andAMBER_RDC_CD
known issues:
unsafe $...$
blocks. I think its an issue to discuss, whether to ignoreunsafe
commands or create a new keyword for disabling RDC on one specific command.Closes #95