tcl/tk: Fix layer toolbar icon synchronization on left-click#508
tcl/tk: Fix layer toolbar icon synchronization on left-click#508Ahmed-Nematallah wants to merge 2 commits into
Conversation
There is a bug where left-clicking a hidden layer in the toolbar successfully makes the layer visible in the layout but fails to update the toolbar icon to the "active" state.
The root cause is a positional argument mismatch in magic::toolupdate. When the Magic C-core issues a callback for a "see" command with two arguments, the third argument (layerlist) is passed as an empty string ("") rather than being omitted. This prevents the Tcl procedure from falling back to its hardcoded default of "none", causing the script to skip the logic that reassigns $yesno to "yes".
This patch changes the default value of $layerlist to "" and updates the conditional check to ensure the state and layer name are correctly reassigned regardless of how the C-core signals the update.
|
Feedback comments inline review. |
| if { $Winopts(${topname},toolbar) == 0 } { return } | ||
|
|
||
| if {$layerlist == "none"} { | ||
| if {$layerlist == ""} { |
There was a problem hiding this comment.
consider if {$layerlist == "" || $layerlist == "none"} {
This seems more defensive programming, backwards compatible and manages both values in the same way. Unless a blank string "" has a specific and independent meaning than "none" ?
There was a problem hiding this comment.
That makes sense. In my testing with "see metal1", I observed an empty string being passed, which matches the default operands in most other Magic procedures. But it is understandable to keep backward compatibility.
(I did briefly wonder if someone might name a layer 'none', but that’s highly unlikely! 😬)
I guess the best approach is keeping the default argument as an empty string, checking for both, and adding a comment indicating you should not call a layer "none"
I'll update the PR.
There was a problem hiding this comment.
Hi @dlmiles, I tested the new version and it works.
Added check for 'none' layer name to ensure backwards compatibility.
|
Pulled and merged to the opencircuitdesign.com database. The github version will update overnight. |
3 participants
There is a bug where left-clicking a hidden layer in the toolbar successfully makes the layer visible in the layout but fails to update the toolbar icon to the "active" state.
The root cause is a positional argument mismatch in magic::toolupdate. When the Magic C-core issues a callback for a "see" command with two arguments, the third argument (layerlist) is passed as an empty string ("") rather than being omitted. This prevents the Tcl procedure from falling back to its hardcoded default of "none", causing the script to skip the logic that reassigns $yesno to "yes".
This patch changes the default value of $layerlist to "" and updates the conditional check to ensure the state and layer name are correctly reassigned regardless of how the C-core signals the update.
This pull request fixes Issue #507