Fix CLI help formatting #2945

newhoggy · 2021-07-20T04:04:57Z

No description provided.

newhoggy · 2021-07-21T09:19:25Z

Before:

Usage: cardano-cli transaction build-raw [--byron-era | --shelley-era |
                                           --allegra-era | --mary-era |
                                           --alonzo-era] (--tx-in TX-IN
                                         [--tx-in-script-file FILE
                                           [
                                             (--tx-in-datum-file FILE |
                                               --tx-in-datum-value JSON VALUE)
                                             (--tx-in-redeemer-file FILE |
                                               --tx-in-redeemer-value JSON VALUE)
                                             --tx-in-execution-units (INT, INT)]])
                                         [--tx-in-collateral TX-IN]
                                         [--tx-out ADDRESS VALUE
                                           [--tx-out-datum-hash HASH]]
                                         [--mint VALUE (--mint-script-file FILE
                                           [
                                             (--mint-redeemer-file FILE |
                                               --mint-redeemer-value JSON VALUE)
                                             --mint-execution-units (INT, INT)])]
                                         [--invalid-before SLOT]
                                         [--invalid-hereafter SLOT]
                                         [--fee LOVELACE]
                                         [--certificate-file CERTIFICATEFILE
                                           [--certificate-script-file FILE
                                             [
                                               (--certificate-redeemer-file FILE |

                                                 --certificate-redeemer-value JSON VALUE)
                                               --certificate-execution-units (INT, INT)]]]
                                         [--withdrawal WITHDRAWAL
                                           [--withdrawal-script-file FILE
                                             [
                                               (--withdrawal-redeemer-file FILE |

                                                 --withdrawal-redeemer-value JSON VALUE)
                                               --withdrawal-execution-units (INT, INT)]]]
                                         [--json-metadata-no-schema |
                                           --json-metadata-detailed-schema]
                                         [--auxiliary-script-file FILE]
                                         [--metadata-json-file FILE |
                                           --metadata-cbor-file FILE]
                                         [--genesis FILE |
                                           --protocol-params-file FILE]
                                         [--update-proposal-file FILE]
                                         --out-file FILE

After:

Usage: cardano-cli transaction build-raw  
            [ --byron-era
            | --shelley-era
            | --allegra-era
            | --mary-era
            | --alonzo-era
            ]
            ( --tx-in TX-IN
              [ --tx-in-script-file FILE
                [ (--tx-in-datum-file FILE | --tx-in-datum-value JSON VALUE)
                  ( --tx-in-redeemer-file FILE
                  | --tx-in-redeemer-value JSON VALUE
                  )
                  --tx-in-execution-units (INT, INT)
                ]
              ]
            )
            [--tx-in-collateral TX-IN]
            [--tx-out ADDRESS VALUE [--tx-out-datum-hash HASH]]
            [ --mint VALUE
              ( --mint-script-file FILE
                [ (--mint-redeemer-file FILE | --mint-redeemer-value JSON VALUE)
                  --mint-execution-units (INT, INT)
                ]
              )
            ]
            [--invalid-before SLOT]
            [--invalid-hereafter SLOT]
            [--fee LOVELACE]
            [ --certificate-file CERTIFICATEFILE
              [ --certificate-script-file FILE
                [ ( --certificate-redeemer-file FILE
                  | --certificate-redeemer-value JSON VALUE
                  )
                  --certificate-execution-units (INT, INT)
                ]
              ]
            ]
            [ --withdrawal WITHDRAWAL
              [ --withdrawal-script-file FILE
                [ ( --withdrawal-redeemer-file FILE
                  | --withdrawal-redeemer-value JSON VALUE
                  )
                  --withdrawal-execution-units (INT, INT)
                ]
              ]
            ]
            [--json-metadata-no-schema | --json-metadata-detailed-schema]
            [--auxiliary-script-file FILE]
            [--metadata-json-file FILE | --metadata-cbor-file FILE]
            [--genesis FILE | --protocol-params-file FILE]
            [--update-proposal-file FILE]
            --out-file FILE

newhoggy · 2021-07-21T09:24:37Z

In order to get the traced version of the output for inspection in a browser, supply CLI_HELP_TRACE=1 in the environment. For example:

CLI_HELP_TRACE=1 cardano-cli transaction build-raw

cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs

catch-21 · 2021-07-28T13:37:13Z

@newhoggy Much improved overall. There's just a couple of things bugging me and causing misalignment. Neither of these are serious.

Only sometimes is there's a space after ( or [, before the option name. I see both ( --tx-in-redeemer-file FILE (with space) and (--tx-in-datum-file FILE (without space). This occurs throughout for all cmds. I think it'd look cleaner if all had the space.
The nested options for --tx-in-script-file are not aligned. The opening bracket for --tx-in-redeemer-file and --tx-in-redeemer-value etc are 1 space ahead of the opening bracket for --tx-in-datum-file. See:

            (--tx-in TX-IN
              [--tx-in-script-file FILE
                [(--tx-in-datum-file FILE | --tx-in-datum-value JSON VALUE)
                  ( --tx-in-redeemer-file FILE
                  | --tx-in-redeemer-value JSON VALUE
                  )
                  --tx-in-execution-units (INT, INT)]])

newhoggy · 2021-07-28T23:12:32Z

Yeah, ideally, I want it to look like this:

           ( --tx-in TX-IN
             [ --tx-in-script-file FILE
               [ (--tx-in-datum-file FILE | --tx-in-datum-value JSON VALUE)
                 ( --tx-in-redeemer-file FILE
                 | --tx-in-redeemer-value JSON VALUE
                 )
                 --tx-in-execution-units (INT, INT)
               ]
             ]
           )

ie. space after the bracket/parens if the group spans more than one line, but no space otherwise. Unfortunately the layout code is fairly complex and I haven't yet worked it out even after spending over a day on it. The whole layout code may need a rewrite to achieve the right effect.

I'm interest in merging this on the basis that it is an improvement.

cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs

moo.html

Comments addressed

Jimbo4350 · 2021-08-02T08:11:05Z

cabal.project

@@ -107,6 +107,18 @@ package cardano-ledger-alonzo-test
 -- --------------------------- 8< --------------------------
 -- Please do not put any `source-repository-package` clause above this line.

+source-repository-package


Can we leave a comment that we want to eventually not rely on our own fork after the relevant PR has been merged?

Jimbo4350 · 2021-08-02T08:12:17Z

cardano-cli/src/Cardano/CLI/Render.hs

+import qualified System.IO.Unsafe as IO
+
+cliHelpTraceEnabled :: Bool
+cliHelpTraceEnabled = IO.unsafePerformIO $ do


I don't have any experience using unsafePerformIO. Is this ok to use here?

Yes this seems to be a safe use of unsafePerformIO .

Yep. This example of unsafePerformIO is fairly normal. unsafePerformIO is used to perform some IO to initialise a global constant. The danger is if the side effect is executed more than once which can happen if the constant is inlined into the callsite, in which case the constant is not so constant, but this is prevent by the NOINLINE pragma. This forces the value to be a thunk that can only be evaluated once.

See documentation on unsafePerformIO: https://hackage.haskell.org/package/base-4.15.0.0/docs/System-IO-Unsafe.html

erikd · 2021-08-02T08:20:32Z

cardano-cli/src/Cardano/CLI/Render.hs

+cliHelpTraceEnabled = IO.unsafePerformIO $ do
+  mValue <- IO.lookupEnv "CLI_HELP_TRACE"
+  return $ mValue == Just "1"
+{-# NOINLINE cliHelpTraceEnabled #-}


Yes we need that.

newhoggy · 2021-08-03T05:38:07Z

bors merge

2945: Fix CLI help formatting r=newhoggy a=newhoggy Co-authored-by: John Ky <john.ky@iohk.io>

* Switch to prettyprinter library * Tracing * Command for printing all help * Option ordering note on build-raw command

iohk-bors · 2021-08-03T06:17:43Z

Canceled.

MarcFontaine

Thanks for fixing the tests.
Looks good to me

newhoggy · 2021-08-03T23:21:34Z

bors merge

iohk-bors · 2021-08-04T00:31:20Z

Build succeeded:

newhoggy force-pushed the fix-cli-3 branch 9 times, most recently from 856d4e2 to 1b3ab31 Compare July 21, 2021 09:13

newhoggy marked this pull request as ready for review July 21, 2021 09:18

newhoggy requested review from DavidEichmann, dcoutts, erikd and Jimbo4350 as code owners July 21, 2021 09:18

newhoggy changed the title ~~Fix cli 3~~ Fix CLI help formatting Jul 21, 2021

newhoggy force-pushed the fix-cli-3 branch from 1b3ab31 to 02c13b2 Compare July 22, 2021 01:40

newhoggy requested review from deepfire, denisshevchenko, jutaro and MarcFontaine as code owners July 22, 2021 05:10

newhoggy force-pushed the fix-cli-3 branch 4 times, most recently from 14ff79f to 74b8291 Compare July 28, 2021 04:10

newhoggy requested a review from catch-21 July 28, 2021 04:16

catch-21 reviewed Jul 28, 2021

View reviewed changes

cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs Outdated Show resolved Hide resolved

catch-21 linked an issue Jul 28, 2021 that may be closed by this pull request

[FR] - Emphasise importance of ordering for Alonzo cli build-raw options #2858

Closed

newhoggy force-pushed the fix-cli-3 branch from 74b8291 to dc6db5b Compare July 29, 2021 00:36

catch-21 reviewed Jul 29, 2021

View reviewed changes

cardano-cli/src/Cardano/CLI/Shelley/Parsers.hs Outdated Show resolved Hide resolved

catch-21 previously requested changes Jul 29, 2021

View reviewed changes

moo.html Outdated Show resolved Hide resolved

newhoggy force-pushed the fix-cli-3 branch 2 times, most recently from d47d057 to 0d7d239 Compare July 30, 2021 15:14

newhoggy requested a review from catch-21 July 30, 2021 15:14

newhoggy force-pushed the fix-cli-3 branch from 0d7d239 to 1bfc9f7 Compare July 30, 2021 15:16

catch-21 approved these changes Jul 30, 2021

View reviewed changes

newhoggy force-pushed the fix-cli-3 branch from 1bfc9f7 to c324a51 Compare July 31, 2021 01:17

Jimbo4350 approved these changes Aug 2, 2021

View reviewed changes

erikd reviewed Aug 2, 2021

View reviewed changes

newhoggy force-pushed the fix-cli-3 branch from c324a51 to d1d4356 Compare August 2, 2021 12:08

newhoggy requested a review from erikd August 3, 2021 05:37

iohk-bors bot added a commit that referenced this pull request Aug 3, 2021

Merge #2945

5abac41

2945: Fix CLI help formatting r=newhoggy a=newhoggy Co-authored-by: John Ky <john.ky@iohk.io>

Improved formatting:

72f6fb2

* Switch to prettyprinter library * Tracing * Command for printing all help * Option ordering note on build-raw command

newhoggy force-pushed the fix-cli-3 branch from d1d4356 to 72f6fb2 Compare August 3, 2021 06:17

MarcFontaine approved these changes Aug 3, 2021

View reviewed changes

iohk-bors bot merged commit b30845f into master Aug 4, 2021

iohk-bors bot deleted the fix-cli-3 branch August 4, 2021 00:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CLI help formatting #2945

Fix CLI help formatting #2945

newhoggy commented Jul 20, 2021

newhoggy commented Jul 21, 2021

newhoggy commented Jul 21, 2021

catch-21 commented Jul 28, 2021 •

edited

Loading

newhoggy commented Jul 28, 2021 •

edited

Loading

Jimbo4350 Aug 2, 2021

Jimbo4350 Aug 2, 2021

erikd Aug 2, 2021

newhoggy Aug 2, 2021 •

edited

Loading

newhoggy Aug 2, 2021

erikd Aug 2, 2021

newhoggy commented Aug 3, 2021

iohk-bors bot commented Aug 3, 2021

MarcFontaine left a comment

newhoggy commented Aug 3, 2021

iohk-bors bot commented Aug 4, 2021

Fix CLI help formatting #2945

Fix CLI help formatting #2945

Conversation

newhoggy commented Jul 20, 2021

newhoggy commented Jul 21, 2021

newhoggy commented Jul 21, 2021

catch-21 commented Jul 28, 2021 • edited Loading

newhoggy commented Jul 28, 2021 • edited Loading

Jimbo4350 Aug 2, 2021

Choose a reason for hiding this comment

Jimbo4350 Aug 2, 2021

Choose a reason for hiding this comment

erikd Aug 2, 2021

Choose a reason for hiding this comment

newhoggy Aug 2, 2021 • edited Loading

Choose a reason for hiding this comment

newhoggy Aug 2, 2021

Choose a reason for hiding this comment

erikd Aug 2, 2021

Choose a reason for hiding this comment

newhoggy commented Aug 3, 2021

iohk-bors bot commented Aug 3, 2021

MarcFontaine left a comment

Choose a reason for hiding this comment

newhoggy commented Aug 3, 2021

iohk-bors bot commented Aug 4, 2021

catch-21 commented Jul 28, 2021 •

edited

Loading

newhoggy commented Jul 28, 2021 •

edited

Loading

newhoggy Aug 2, 2021 •

edited

Loading