Skip to content

QT: Add 'copy full transaction details' option #7628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jonasschnelli merged 1 commit into from Mar 8, 2016
Merged

QT: Add 'copy full transaction details' option #7628

jonasschnelli merged 1 commit into from Mar 8, 2016

Conversation

@ghost
Copy link

@ghost ghost commented Mar 1, 2016

Adds feature from issue #7484

@ghost
Copy link
Author

ghost commented Mar 1, 2016

No output has been received in the last 10 minutes, this potentially indicates a stalled build or something wrong with the build itself.

That's a bit odd. My pull request compiled fine on every other job in the Travis CI build... so I'm not sure what to make of this.

@maflcko
Copy link
Member

maflcko commented Mar 1, 2016

No worries, it is a know issue unrelated to your changes.

@ghost
Copy link
Author

ghost commented Mar 1, 2016

ah, I see. what a relief

@laanwj
Copy link
Member

laanwj commented Mar 2, 2016

Restarted travis, it should pass now.

@paveljanik
Copy link
Contributor

paveljanik commented Mar 3, 2016

ACK https://github.com/ericshawlinux/bitcoin/commit/fff0dcc4a42f384fa9e7778fa0241f3448454658

Minor NIT: empty '()' on addresses without labels. Can be tested beforehand...

For unconfirmed transaction:

3/3/16 08:05 Unconfirmed () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

4 confirms:

3/3/16 08:05 Confirming (4 of 6 recommended confirmations) () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

Confirmed:

3/3/16 08:05 Confirmed (6 confirmations) () mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

With label:

3/3/16 08:05 Confirmed (6 confirmations) (Test label for #7628) mjHi7MpmjKzez9KY8zL6sSQVtBpqsaB9LF 2.60000000

The debug log contains this on OS X (the same warning as with Copy raw transaction:

2016-03-03 07:10:46 GUI: Data set on unsupported clipboard mode. QMimeData object will be deleted.

@paveljanik
Copy link
Contributor

paveljanik commented Mar 3, 2016

Off-topic here, but anyway: it would be nice to be able to add label to the address directly from the main window, in Recent transactions.

jonasschnelli
jonasschnelli reviewed Mar 3, 2016
View reviewed changes
src/qt/transactiontablemodel.cpp Outdated
Copy link
Contributor

@jonasschnelli jonasschnelli Mar 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: what about only append L620 to L622 if !rec->address.empty()? I guess most users have couple of transaction without according address labels.

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Mar 3, 2016

Tested ACK modulo nit.

Agree with @paveljanik about the () and the warning. Although the console-out warning is unrelated to this PR. If it can be fixed, it could be done in a different PR.

Binaries: https://bitcoin.jonasschnelli.ch/pulls/7628/

4 Examples

3/3/16 13:19 Unconfirmed ()  -0.00003840
3/3/16 13:24 Unconfirmed (Yes! A testlabel.) mgY7FaksgbtrNPVJKzJBrKTG3Ayfqddc8R -1.00003840
2/22/16 09:27 Immature (6 confirmations, will be available after 101) () mr5C9MyoswwkTzFzwys6LfS5TdrX6u8xRo 0.39062500
2/22/16 09:27 Confirmed (143 confirmations) () mr5C9MyoswwkTzFzwys6LfS5TdrX6u8xRo 0.78125000

Two conceptual ideas for further improvements:

  1. Would adding the transaction type make sense? In/Out/Mined? Or is the +/- in the balance enough, at least coinbase transaction are indistinguishable against "other" incoming txes?
  2. If I select a row and hit Ctrl-C, currently, it does copy the cell-content I have last clicked (only date or only type, etc.). I think it would be nice to copy the "full details" if one presses Ctrl-C while having a row selected.

jonasschnelli
jonasschnelli reviewed Mar 6, 2016
View reviewed changes
src/qt/transactiontablemodel.cpp Outdated
Copy link
Contributor

@jonasschnelli jonasschnelli Mar 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use details.append(tr("(no label)")+" "); here to allow translations.

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Mar 6, 2016

Can you squash your commits?

@ghost
Copy link
Author

ghost commented Mar 7, 2016

Ok

QT: Add 'copy full transaction details' option
b51ed40 
Adds feature from issue #7484

modifies the ctrl-c binding to copy full transaction details in transaction view.

Added translation
@jonasschnelli
Copy link
Contributor

jonasschnelli commented Mar 8, 2016

Tested ACK b51ed40.

@jonasschnelli jonasschnelli merged commit b51ed40 into bitcoin:master Mar 8, 2016
jonasschnelli added a commit that referenced this pull request Mar 8, 2016
@jonasschnelli
Merge #7628: QT: Add 'copy full transaction details' option
87d6562 
b51ed40 QT: Add 'copy full transaction details' option (Eric Shaw)
@paveljanik paveljanik mentioned this pull request Mar 30, 2016
Closed
codablock pushed a commit to codablock/dash that referenced this pull request Sep 5, 2017
@jonasschnelli @codablock
Merge bitcoin#7628: QT: Add 'copy full transaction details' option
1448da5 
b51ed40 QT: Add 'copy full transaction details' option (Eric Shaw)
UdjinM6 pushed a commit to dashpay/dash that referenced this pull request Sep 7, 2017
@codablock @UdjinM6
Backport Bitcoin Qt/Gui changes up to 0.14.x part 1 (#1614)
690cb58 
* Merge pull request bitcoin#7154

a3c3ddb [Qt] add InMempool() info to transaction details (Jonas Schnelli)

* Merge pull request bitcoin#7218

fa5769e [qt] Fix misleading translation (MarcoFalke)
fa8c8d7 torcontrol debug: Change to a blanket message that covers both cases (MarcoFalke)

* Merge pull request bitcoin#7255

6fd0a07 Remove hardcoded fee from CoinControl ToolTip (fanquake)
5fdf32d Replace some instances of formatWithUnit with formatHtmlWithUnit (fanquake)

* Merge pull request bitcoin#7263

a5a0831 Double semicolon cleanup. (21E14)

* Merge pull request bitcoin#7334

fa989fb [qt] coincontrol workaround is still needed in qt5.4 (fixed in qt5.5) (MarcoFalke)

* Merge pull request bitcoin#7329

9d263bd Typo fixes in comments (Chris Wheeler)

* Merge bitcoin#7396: [Qt] Add option to increase/decrease font size in the console window

43abb02 [Qt] Add a new chevron/arrow icon for the console prompt line (Jonas Schnelli)
56c9e66 [Qt] keep scroll position in GUI console after changing font size (Jonas Schnelli)
3a3a927 [Qt] Add option to increase/decrease font size in the console window (Jonas Schnelli)

* Merge bitcoin#7628: QT: Add 'copy full transaction details' option

b51ed40 QT: Add 'copy full transaction details' option (Eric Shaw)

* Merge bitcoin#7668: Fix history deletion bug after font size change

21e45a0 Fix history deletion bug after font change (Andrew C)

* Copy/Move font size related icons into theme folders

* Use formatTxDate for date/time in TxPlainTextRole
@hebasto hebasto mentioned this pull request Mar 26, 2021
Closed
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

GUI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maflcko @laanwj @paveljanik @jonasschnelli