[Qt] Add spend all button to the SendCoinsDialog #11098

Related to: https://github.com/bitcoin/bitcoin/issues/11033

I don't like the dividing equally between outputs behaviour, I think the button should only be possible on single entry spends, makes little sense to me otherwise. From discussion in #11033, @laanwj didn't just want a button added for appearance reasons. Also nits, please update to snake_case and modify braces as per style guideline.

I don't like the dividing equally between outputs behaviour, I think the button should only be possible on single entry spends, makes little sense to me otherwise.

Don't use the button with multiple outputs if you don't like it, no reason to stop other people from dividing their balance between multiple recipients. The way the SendCoinsDialog works right now assumes that there can be any number of recipients.

From discussion in #11033, @laanwj didn't just want a button added for appearance reasons.

Feel free to suggest a better option. I put the button in an area so that it doesn't take up any vertical space and mess up the appearance of the entire page. It would be possible to put something on the SendCoinEntry(s) instead and send a signal to the SendCoinsDialog instance for example but that was more cluttered visually and in code when I tried it.

Also nits, please update to snake_case and modify braces as per style guideline.

You'll have to point out where you want me to use snake case...

You'll have to point out where you want me to use snake case...

See developer notes.

The best assumption that I can make from your response is that you want me to use snake case for variable names. I see that is in the developer notes now, but it is certainly not in line with most of the code that I see in Bitcoin core. Am I correct that we are now going to diverge from the style in the existing codebase, and require all new code to look different, creating a big mess of many different styles?

I don't know how much value a style guideline has besides maintaining clarity and uniformity of code, aren't we throwing that out the window here?

@CryptAxe Yes, use snake case for all new code. Please follow the developer guidelines for all new code, regardless of the style currently in the codebase. Since the codebase changes a lot, the style of the codebase will shift over to the new style as things are changed and replaced.

Why snake case, what led to that change? Who even made this decision and why would we diverge from the existing code in the first place? If this rule is followed we are making Bitcoin's code worse, it will be a mix of several styles (the new one being especially ugly in my opinion). The point of style guidelines are to maintain clarity and uniformity. There is no reason for Bitcoin to have style guidelines if they are going to change all the time. Do we really think 100% of the codebase is going to be re-written in the new style, or is it more likely that we will end up with an ugly mess as a result of this?

Also I'm not trying to roast anyone who has commented here, I'm addressing the broader developer community with my questions..

Why snake case, what led to that change? Who even made this decision and why would we diverge from the existing code in the first place?

Some group of people did, git blame can point you in the right direction. But who cares, for me consistency wins.

Do we really think 100% of the codebase is going to be re-written in the new style

Eventually.

or is it more likely that we will end up with an ugly mess as a result of this?

I guess it is a mess because of the early lack of rules to follow and time to enforce those rules. Even nowadays, there is lot of new code that doesn't met those rules.

Concept ACK. I think the send all button needs to be in the send-to-box context (not in the global). Ideally close to where you enter the amount (it's mostly related to that IMO).

I think it should be [amount] [denomination] [send-all] [subtract-from-amount].

Maybe we should call it not send-all because the max amount may change after you have pressed the button == you no longer sending "all". Maybe something like "select all", or "use max available" makes more sense.

<img width="1067" alt="bildschirmfoto 2017-09-07 um 10 28 10" src="https://user-images.githubusercontent.com/178464/30176575-c43a67f6-93b7-11e7-8c2c-ec8b2156f820.png">

Do you think a checkbox would be good for that? It looked a bit weird having two of them next to eachother. I can't come up with anything prettier besides a QLable with an icon people can click on, but we would need to add an icon.

On Sep 7, 2017 10:33 AM, "Jonas Schnelli" notifications@github.com wrote:

Concept ACK. I think the send all button needs to be in the send-to-box context (not in the global). Ideally close to where you enter the amount (it's mostly related to that IMO).

I think it should be [amount] [denomination] [send-all] [subtract-from-amount].

Maybe we should call it not send-all because the max amount may change after you have pressed the button == you no longer sending "all". Maybe something like "select all", or "use max available" makes more sense.

[image: bildschirmfoto 2017-09-07 um 10 28 10] https://user-images.githubusercontent.com/178464/30176575-c43a67f6-93b7-11e7-8c2c-ec8b2156f820.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/11098#issuecomment-327869628, or mute the thread https://github.com/notifications/unsubscribe-auth/AHu1Ntv3bIPNHOe5zfA5y35LPhibq5xdks5sgCjZgaJpZM4O8iEI .

What's wrong with using a button like you're currently doing? Why are you talking about a checkbox?

Well I can but adding a push button where Jonas has the arrow pointing would take up more vertical space and mess up the layout a bit. Should I make some screenshots showing different options?

On Sep 7, 2017 7:37 PM, "MeshCollider" notifications@github.com wrote:

What's wrong with using a button like you're currently doing? Why are you talking about a checkbox?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/11098#issuecomment-327983132, or mute the thread https://github.com/notifications/unsubscribe-auth/AHu1NuAHIwuGVjPsnlTA719fJ9OSWAtKks5sgKiAgaJpZM4O8iEI .

Maybe something like "select all", or "use max available" makes more sense

Agree with @jonasschnelli, and that "button" should fill the amount field and toggle the checkbox. And I think it works well with multiple outputs: So when pressed the amount field will have the available amount minus other input amounts.

#11316 which improved this was merged so I'll close it.