magic icon indicating copy to clipboard operation
magic copied to clipboard

PR#340 alternative ? flush -noconfirm .. exit exit_status -noconfirm

Open dlmiles opened this issue 1 year ago • 5 comments

@wulffern please advise, test as alternative to PR#340

Documented hidden/secret exit [-noconfirm] option to disable exit prompt Added exit [exit_status] option to send exit_status number Added flush -noconfirm option to disable flush prompt

I have tested the 'exit' patches.

I have not yet tested the 'flush' patch. Still comment back when testing complete on my side but would appreciate feedback if it closes your concern.

dlmiles avatar Oct 09 '24 22:10 dlmiles

@dlmiles Yup, I can confirm that the flush -noprompt does what I want. Thanks.

It's a bit more finicky to use since I need to know that the -noprompt option is in the Magic version.

Magic throws an error if I use -noprompt in a Magic version that does not have it, but with a "flush_all" script like the below it works fine.

set values [ cellname list all ]
set mrev [lindex [lreverse [split [exec magic --version] "."]] 0 ]
foreach x $values {
    if {${mrev} > 495} {
        flush $x -noprompt
    } else {
        flush $x
    }
}

PS: I could not find the revision in any global tcl variable, nor was I able to redirect "version" to a variable, but I'm sure there is a more elegant way to do the above.

wulffern avatar Oct 10 '24 07:10 wulffern

@wulffern : catch {flush -noprompt} should work around the backwards-compatibility failure, or maybe

if {[catch {flush -noprompt}]} {flush}

RTimothyEdwards avatar Oct 10 '24 13:10 RTimothyEdwards

@RTimothyEdwards Did you want a revision using Lookup() ? not clear as it PR marked approved.

@wulffern maybe you can setup $ENV{'MAGIC_VERSION'} before magic is run, to TCL inherits envars, like in a startup wrapper shell script ? This way it is always available. This isn't much difference to how you are doing it now really. Maybe there should be a patch to make such a thing available from TCL an auto-populated variable.

Maybe it can be run as flush $x -noprompt and if this errors, too many args, it just retries the command without -noprompt. This may not be a possible if $x is not present, as -noprompt' maybe processed as a cellname` which will probably also error.

Difficult to provide a backward compatible mechanism that isn't as finicky, as least you can make it into myflush TCL proc to clean / hide this extra work?

Your original patch with an environment variable set from inside (or maybe inherited from outside) might be better reworked with the batch_mode feature, such that you can start up in GUI mode with batch_mode and it has the effect of making all interactive decision dialogs to cause error termination. It also make sense for $ENV{'MAGIC_BATCH_MODE'} to also be a thing so scripts a runtime. Maybe this is already possible.

Which then necessitates an option like -noprompt for every command that might present a dialog, which allows the command to succeed. This is my use case, I am looking to understand how some test cases maybe prepared that operate in GUI mode, but I need a way to manage dialog interactions and terminate with exit_status. Hence this PR.

dlmiles avatar Oct 10 '24 13:10 dlmiles

@dlmiles : I approved it because the code is usable as-is, but if you can change it to use Lookup() I'd appreciate it.

FYI: You can use the version command in magic to get the version; it does not need to come from an external variable. Although a version --list option would be nice to get just the version numbers back as a Tcl list instead of text.

RTimothyEdwards avatar Oct 10 '24 18:10 RTimothyEdwards

I have checked over the Lookup() methods and the usages in the codebase I can't quite see an obvious way all the requirements can be met :

  • allow -option to be any order (ok regular Lookup usage does this part)
  • continue to ensure -nodef is the same as the longer form -nodeference but also -nodefer etc.. to ensure a users script doesn't break (this is the main blocker)
  • extract the [exit_status] option, is this allowed to be in any position ? (unclear how the remaining arguments left in the list, unclaimed by any Lookup is extracted into a canonicalized form argv[]) so the arg count in this category can be seen, to allow exit -noprompt 42 but also check for invalid exit -noprompt 42 0
  • error on weird usage like exit 1 -noprompt -noprompt -nodef -nodeference maybe this is a minor one

Re current patch having fixed order, sure I added in a way that should not break any current users scripts and documentation indicates correct order to make it as easy to discover and get working as possible.

dlmiles avatar Oct 16 '24 05:10 dlmiles

Pulled and merged into the opencircuitdesign.com code base.

RTimothyEdwards avatar Dec 26 '24 17:12 RTimothyEdwards

Not sure what happened to this change set it did not make it into master

Instead there is 772bfe2f71f31bf8e0254fe685c8bc65e0829709 from Carsten (and the initial version that was using a non-standard method to activate the feature)

So I rebase 772bfe2f with a revert ahead of the 3 commits here.

dlmiles avatar Jan 03 '25 07:01 dlmiles

@RTimothyEdwards No re-open issue button here.

Just in case it is not clear from issue information which 4 commits they can be seen here https://github.com/RTimothyEdwards/magic/compare/master...dlmiles:magic:master-upstream-20241009-noconfirm the new HEAD is 0a27045

Update: This is also the cause of the WASM build failures in GHA. the commit from #340 exposes magicinterp without ifdef for WASM use. So the 4 commits now here will resolve that as well.

dlmiles avatar Jan 03 '25 07:01 dlmiles