slimcoin-project / pacli

Simple CLI PeerAssets client (extended version).
GNU General Public License v3.0
0 stars 0 forks source link

Yet another flag for trasaction show command? #17

Closed buhtignew closed 9 months ago

buhtignew commented 9 months ago

This issue refers to the following your post: https://github.com/slimcoin-project/pacli/issues/9#issuecomment-1936865185

Would it make sense to introduce a short form flag variant for -d in case of transaction show LABEL -d? I mean maybe it would be comfortable to have the possibility to get transaction id using label. I tried with transaction show LABEL -d -q but got more concise version of -d flag output.

If we introduce such a flag it could be also implemented for transaction list -n as well.

d5000 commented 9 months ago

I don't fully understend: do you mean a variant where you enter the label of the transaction and get (only) the TXID?

This can of course be done. I haven't implemented this because for the original intent of this command (to store partly signed transactions for the DEX) it doesn't make sense because the TXID changes after it becomes fully signed. But if you think there may be an use case - it's perhaps one line of code or two.

buhtignew commented 9 months ago

I think it would make sense for those who'd use the labels for their own purposes. In such a case I guess they may need a tool to know what is actually the transaction that is behind the label they know.

d5000 commented 9 months ago

Ok, implemented in both transaction show and transaction list. Commit 173f127

Usage: pacli transaction show LABEL -t/--txid pacli transaction list --named -t/--txids or pacli transaction list -t/--txids (can be combined with addresses too)

buhtignew commented 9 months ago

pacli transaction show LABEL -t/--txid

I've tested this command and it works as expected.

Then I've tested transaction show f4e26f3a14a7fbe7ea45bd60ad9005ebbaa36a3cecd864c7f3ef517c5fe27c86 -t and got f4e26f3a14a7fbe7ea45bd60ad9005ebbaa36a3cecd864c7f3ef517c5fe27c86.

Then I've tested transaction show f4e26f3a14a7fbe7ea45bd60ad9005ebbaa36a3cecd864c7f3ef517c5fe27c86 -d and got the JSON string.

Then I've tested transaction show LABEL -d And got the JSON string.

The following lines of the help have made me think that there was a way to output the HEX of the transactions using the -d flag but I've was not able to: Shows a transaction stored in the extended config file, by label, as HEX or JSON string. Shows any transaction's content, as HEX or JSON string. -d, --decode: Show transaction in JSON format (default: hex format). So I was wondering what should be done to output the HEX, if necessary, or whether we should change the above lines in the help.

I also think there is a typo in the line pacli transaction show TXID -a of the help, the -a flag should be probably edited into -s.

_

pacli transaction list --named -t

Tested, works as expected.

_

pacli transaction list -t

Tested, the output seems to be similar to HEX's. Would it make sense to put the TXIDs into braces or to differentiate the output in any other way from the HEX?

d5000 commented 9 months ago

So I was wondering what should be done to output the HEX, if necessary, or whether we should change the above lines in the help.

(Edited) The hex string is shown when -d is not given. I'll clarify this more in the help text.

I also think there is a typo in the line pacli transaction show TXID -a of the help, the -a flag should be probably edited into -s.

Thanks, that was because the option was temporarily called anatomy and now is called structure again. It's corrected for the next update (not the big one with the new flags).

pacli transaction list -t [...]Would it make sense to put the TXIDs into braces or to differentiate the output in any other way from the HEX?

I have to think about how this could be improved, but would like to not have to add additional braces. I get the problem as the "pretty print" format makes the hex string look like several transaction IDs. So I think what I'll do is to change the output format of the hex strings.

Update: All done in commit 70ecd50.

Update 2: For consistency, the -t option was renamed to -i, like in transaction list.

(Sorry, accidentally edited you previous comment. Have restored it but probably the formatting isn't correct).

buhtignew commented 9 months ago

The hex string is shown when -d is not given

Of course! How could I've forgotten it?

I'll clarify this more in the help text.

Now it's much more clear.