Matsl rsw fix ebut act #462
Conversation
|
I agree that as hbut:act does a check that the button sent in is valid, so
too should ibut:act and ebut:act (the new versions you wrote).
…On Fri, Feb 2, 2024 at 4:35 PM Mats Lidell ***@***.***> wrote:
@matsl <https://github.com/matsl> requested your review on: #462
<#462> Matsl rsw fix ebut act.
—
Reply to this email directly, view it on GitHub
<#462 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE5WPD6UA6RMHHIN57YLH63YRVL3PAVCNFSM6AAAAABCXKS6ROVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGY4DSNZQHE3DOOI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
What do you think of also checking when no buttton is sent in that hbut:current is of the right type? In their simple form now calling ibut:act with no args but with hbtut:current being an ebut works fine. Should it give an error message? Taking a step back. Is not calling with no args and then getting hbut:current to kick in a bit of a code smell? If you are writing code should you not be able through other more clear ways decide what button you want to act on so that you don't need to depend on hbut:current? I'm thinking the methods are either called interactively, and user is then prompted for which but to call, or they should be called with an argument? Calling non-interactive with no argument would be an error case? wdyt? |
|
Have to discuss this on the phone. Too involved for me to think through it
in a message right now. Trying to do some work on HyRolo to eliminate any
issues so we can ship. -- Bob
…On Sat, Feb 3, 2024 at 10:51 AM Mats Lidell ***@***.***> wrote:
I agree that as hbut:act does a check that the button sent in is valid, so
too should ibut:act and ebut:act (the new versions you wrote).
What do you think of also checking when no buttton is sent in that
hbut:current is of the right type? In their simple form now calling
ibut:act with no args but with hbtut:current being an ebut works fine.
Should it give an error message?
Taking a step back. Is not calling with no args and then getting
hbut:current to kick in a bit of a code smell? If you are writing code
should you not be able through other more clear ways decide what button you
want to act on so that you don't need to depend on hbut:current? I'm
thinking the methods are either called interactively, and user is then
prompted for which but to call, or they should be called with an argument?
Calling non-interactive with no argument would be an error case? wdyt?
—
Reply to this email directly, view it on GitHub
<#462 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE5WPD4LK7RU423OCCP5WDTYRZMIXAVCNFSM6AAAAABCXKS6ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVGM3DKMBXGE>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
No problem. You should keep focused on fixing the known "old" things. We can keep this as is for now and fix it post the release. Or I can add a check that guards the functions from running a but of the other type and use that both for a supplied arg and for hbut:current. We can leave the more deep discussion to later to not waste time on that now. |
matsl
force-pushed
the
matsl-rsw-fix-ebut-act
branch
from
924e3e9 to
dfa9972
Compare
|
@rswgnu Added check on type of but and corresponding test cases. PTAL |
matsl
force-pushed
the
matsl-rsw-fix-ebut-act
branch
from
dfa9972 to
4bf1bc3
Compare
|
@rswgnu Did an update on the error message, renamed the parameters and added a test for when there is no hbut:current. PTAL |
What
Rename i- ebut:act to act-label and create new act functions.
Why
The old functions takes a label and to be more align with the hbut:act they should take a button. So the old functions have been moved away and new ones have been created. Two very simple tests have been added.
Note
These function are not used a lot. The work horse is hbut:act that centralizes button action. So I'm not sure how well used these will be since you can call hbut:act in most cases just as well. In fact it its current version there is no check that hbut:current actually is of the right type. Should that be added? Anyway, not sure the use case adds that much, but makes the API more complete.