hyperbole
hyperbole copied to clipboard
rswgnu
Matsl rsw fix ebut act
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.
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 https://github.com/rswgnu/hyperbole/pull/462 Matsl rsw fix ebut act.
— Reply to this email directly, view it on GitHub https://github.com/rswgnu/hyperbole/pull/462#event-11689709679, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE5WPD6UA6RMHHIN57YLH63YRVL3PAVCNFSM6AAAAABCXKS6ROVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGY4DSNZQHE3DOOI . You are receiving this because your review was requested.Message ID: @.***>
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?
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 https://github.com/rswgnu/hyperbole/pull/462#issuecomment-1925365071, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE5WPD4LK7RU423OCCP5WDTYRZMIXAVCNFSM6AAAAABCXKS6ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVGM3DKMBXGE . You are receiving this because your review was requested.Message ID: @.***>
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
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.
@rswgnu Did an update on the error message, renamed the parameters and added a test for when there is no hbut:current. PTAL