ask() should take a default #70

Closed
jmthomas opened this Issue Feb 20, 2015 · 10 comments

Comments

Projects
None yet
2 participants
@jmthomas
Member

jmthomas commented Feb 20, 2015

User's have requested that ask() take a default value as many times they are asking for multiple parameters but only one actually changes. The current API is ask(question, allow_blank = false, password = false). Password was just added in 3.2.0. I don't want to break the API but I feel like the default parameter should be second as it's more likely to be used than blank and password. I also think blank is least likely and should be last. Should we create a new method "ask_default" and eventually switch ask to it after a deprecation warning? Or do we continue to simply tack on parameter to the end of methods without any though of usage and consistency?

@jmthomas

This comment has been minimized.

Show comment
Hide comment
@jmthomas

jmthomas Feb 20, 2015

Member

We might also want to allow message_box() to take a default for which button is selected so the user can just hit enter and select the default. That one is easier because the API is message_box(string, buttons) and default can just be added to the end.

Member

jmthomas commented Feb 20, 2015

We might also want to allow message_box() to take a default for which button is selected so the user can just hit enter and select the default. That one is easier because the API is message_box(string, buttons) and default can just be added to the end.

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Feb 20, 2015

Member

I'm ok with changing the ask API and making default the second argument. We can handle it short term, by detecting if the second parameter is a string or a boolean, and assuming which API they are using based on that. If using the old API we can output a deprecation warning and then remove it in a later release.

Member

ryanatball commented Feb 20, 2015

I'm ok with changing the ask API and making default the second argument. We can handle it short term, by detecting if the second parameter is a string or a boolean, and assuming which API they are using based on that. If using the old API we can output a deprecation warning and then remove it in a later release.

@jmthomas

This comment has been minimized.

Show comment
Hide comment
@jmthomas

jmthomas Feb 20, 2015

Member

I think that would work. I'm also going to change the behavior of the Cancel button on dialogs. If you click Cancel the script just pauses. At that point the user can click Go or Stop using the usual buttons. It saves the additional dialog which isn't really buying anything.

Member

jmthomas commented Feb 20, 2015

I think that would work. I'm also going to change the behavior of the Cancel button on dialogs. If you click Cancel the script just pauses. At that point the user can click Go or Stop using the usual buttons. It saves the additional dialog which isn't really buying anything.

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Feb 20, 2015

Member

Ok. removing the second dialog will break autohotkey so make sure you update that.

Member

ryanatball commented Feb 20, 2015

Ok. removing the second dialog will break autohotkey so make sure you update that.

@jmthomas

This comment has been minimized.

Show comment
Hide comment
@jmthomas

jmthomas Feb 20, 2015

Member

Will do.

Member

jmthomas commented Feb 20, 2015

Will do.

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Feb 20, 2015

Member

Actually let's make the second parameter to ask be "default_or_allow_blank". Behavior:
If true or false - same as allow_blank, default is ''
If a string, then the string is the default and blank is not allowed.
Does that work? Basically the same except is you give a default then blank is not allowed

Member

ryanatball commented Feb 20, 2015

Actually let's make the second parameter to ask be "default_or_allow_blank". Behavior:
If true or false - same as allow_blank, default is ''
If a string, then the string is the default and blank is not allowed.
Does that work? Basically the same except is you give a default then blank is not allowed

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Feb 20, 2015

Member

I think the purpose of allow_blank was to have the software inject a default if blank

Member

ryanatball commented Feb 20, 2015

I think the purpose of allow_blank was to have the software inject a default if blank

@jmthomas

This comment has been minimized.

Show comment
Hide comment
@jmthomas

jmthomas Feb 20, 2015

Member

Sounds good.

Member

jmthomas commented Feb 20, 2015

Sounds good.

@ryanatball ryanatball added the feature label Feb 20, 2015

@ryanatball

This comment has been minimized.

Show comment
Hide comment
@ryanatball

ryanatball Feb 20, 2015

Member

Do you want this in the release today?

Member

ryanatball commented Feb 20, 2015

Do you want this in the release today?

@jmthomas

This comment has been minimized.

Show comment
Hide comment
@jmthomas

jmthomas Feb 20, 2015

Member

Sure, let me take a stab at it this morning. Worse part will probably be AHK.

Member

jmthomas commented Feb 20, 2015

Sure, let me take a stab at it this morning. Worse part will probably be AHK.

@ryanatball ryanatball added this to the v3.2.1 milestone Feb 20, 2015

jmthomas pushed a commit that referenced this issue Feb 20, 2015

jmthomas pushed a commit that referenced this issue Feb 20, 2015

@jmthomas jmthomas closed this Feb 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment