cc @zerosnacks assigned to #9697

Generally supportive on the approach, defaulting to true also makes sense here

Generally supportive on the approach, defaulting to true also makes sense here

I'd suggest having args like --snapshot-no-record and --snapshot-no-check that would disable default behavior if passed, or something along these lines, would that work?

the environment variable FORGE_SNAPSHOT_CHECK is already established. adding an additional FORGE_SNAPSHOT_NO_CHECK seems like it could be confusing. interactions between the env and the flags become unclear.

i made the FORGE_SNAPSHOT_CHECK change because

• using a clap flag seemed simple and more consistent than checking the env directly in the conditional
• it collected all the available options and env var access in the clap options block
• the current behavior simply conditions on presence, so something like FORGE_SNAPSHOT_CHECK="false" is interpreted as a truthy value!!

but now i see that also raises many questions, and it is not entirely consistent with the behavior of my added flag.

i will revert changes to FORGE_SNAPSHOT_CHECK and create an issue describing its present behavior. it may be good to resolve that, and all the questions there, before adding a new flag.

i will revert changes to FORGE_SNAPSHOT_CHECK and create an issue describing its present behavior. it may be good to resolve that, and all the questions there, before adding a new flag.

added the gas_snapshot_check config entry + flag + suggested change to FORGE_SNAPSHOT_CHECK here: #9791

thanks for working on this, wound up with other tasks on my plate and wasn't able to make time

the emit behavior still remains to be implemented? will leave this open for now, might get to it with free time this week

@turbocrime No worries! Feel free to ping me if you run into (time) issues / have questions

updated to tip of main, now using config, env var, and a flag, following the pattern @zerosnacks used in #9791

new config field: gas_snapshot_emit, default true (present behavior)
new env var: FORGE_SNAPSHOT_EMIT, overrides config
new cli flag: --gas-snapshot-emit=<bool>, overrides config and env

tests feel verbose, could possibly be improved, but i believe they confirm function.

ready for re-review

hopefully i can come back at some point to expand docs like foundry-rs/book#1407 but i've got limited time now.

Thanks @turbocrime! Implementation and tests look good

hopefully i can come back at some point to expand docs like foundry-rs/book#1407 but i've got limited time now.

Added the new flag to the book foundry-rs/book#1442 :)