Fixing outputFormatter and standardising STDOUT

[addievo]

Description

This PR builds upon #45

And aims to fix some issues which were not resolved in their entirety in #45.

These mainly include:

1. Simplification of outputTableFormatter.
2. Adding quotes back on stdout.
3. Encode non-printable characters except spaces.

Fixed Issues

1. Fixes CLI polish and fixes for deployment #44

Tasks

• 1. Tidy up outputTableFormatter.
• 2. Adding double quotes back on STDOUT.
• 3. Encoding non-printable chars except spaces on STDOUT.
• 4. Define and implement new encodeEscaped and encodeEscapedWrapped API for output character escape encoding

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

Legend

[tegefaulkes]

The code doesn't build. That needs to be fixed before merging.

It was broken in #45 too so I'm not sure why that was merged. You need to follow the merge checklist because it's there to catch this kind of problem. But it's fully ticked right now even though the build fails, why is that?

[CMCDragonkai]

There was prior review commentary located here: #45 (comment).

@amydevs best to refer to the prior comments that @tegefaulkes made too.

[CMCDragonkai]

This is conflicting with staging? You may need a rebase?

And also can you provide previews of what all these commands look like now?

[amydevs]

@amydevs are you going to change the vault ID generation?

It doesn't look right to me, the original code never encoded the tabs.

no, what i'm saying is that a lot of the commands were using \t before for spacing.

like this

data.push(
              `${vaultListMessage.vaultName}:\t${
                vaultListMessage.vaultIdEncoded
              }`,
            );

I've replaced it with spaces instead

data.push(
              `${vaultListMessage.vaultName}:${' '.repeat(4)}${
                vaultListMessage.vaultIdEncoded
              }`,
            );

ID generation never generated characters that had to be escaped.

[amydevs]

https://stackoverflow.com/questions/20585729/how-to-detect-if-nodes-process-stdout-is-being-piped

We might want to never escape when the output is being redirected btw, this will have to be done in another pr at some point if it is needed

[amydevs]

should be done, just wrote up new api for encoding escape characters. Please refer to this PR if it comes up

[CMCDragonkai]

What if there's other kinds non-printable characters? Usually when quoted in JS, they appear as slash unicodes. Also I was wondering if we could have just used the standard console.log and captured it's output somehow. JS seems to have native quoting when it comes to strings. Did you have a look?

[amydevs]

What if there's other kinds non-printable characters? Usually when quoted in JS, they appear as slash unicodes. Also I was wondering if we could have just used the standard console.log and captured it's output somehow. JS seems to have native quoting when it comes to strings. Did you have a look?

native quoting only applies to the browser console, not the STDOUT in Node.JS

At the moment, I am also turning those \u characters into escaped versions