Skip to content
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

Add Markdown output format to the CLI #17697

Merged
merged 2 commits into from
Aug 23, 2023
Merged

Add Markdown output format to the CLI #17697

merged 2 commits into from
Aug 23, 2023

Conversation

nineinchnick
Copy link
Member

Description

Add Markdown output to avoid having to use external tools like csv2md.

Example usage:

./client/trino-cli/target/trino-cli-419-SNAPSHOT-executable.jar --output-format MARKDOWN \
  --execute="select * from (values (1234567, 'some longer strin'), (123, 'aa')) as t(one, two)"

Prints:

|     one | two               |
| -------:| ----------------- |
| 1234567 | some longer strin |
|     123 | aa                |

where the default ALIGNED format looks like:

   one   |        two        
---------+-------------------
 1234567 | some longer strin 
     123 | aa                
(2 rows)

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label May 30, 2023
@nineinchnick
Copy link
Member Author

I also tried using the CommonMark library, but its text renderer is lacking: master...nineinchnick:trino:cli-commonmark-output

wendigo
wendigo reviewed Jul 6, 2023
View reviewed changes
client/trino-cli/pom.xml Outdated Show resolved Hide resolved
wendigo
wendigo requested changes Jul 6, 2023
View reviewed changes
Copy link
Contributor

@wendigo wendigo left a comment

Choose a reason for hiding this comment

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

LGTM % comments

@nineinchnick nineinchnick force-pushed the cli-markdown-output branch from 65572d1 to 43ca4c8 Compare July 17, 2023 08:03
@nineinchnick
Copy link
Member Author

nineinchnick commented Jul 17, 2023 •
edited
Loading

@wendigo ready for another round

@wendigo
Copy link
Contributor

wendigo commented Jul 17, 2023

I've tested rendering in various markdown editors and it looks good.

@nineinchnick nineinchnick force-pushed the cli-markdown-output branch 4 times, most recently from da89953 to 7b42f2d Compare July 20, 2023 19:59
@github-actions github-actions bot added the docs label Jul 20, 2023
wendigo
wendigo requested changes Jul 21, 2023
View reviewed changes
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java
private final List<Align> alignments;
private final Writer writer;

private boolean headerOutput;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: headerRendered

Copy link
Contributor

Choose a reason for hiding this comment

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

bump

client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Outdated Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Outdated Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Outdated Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Outdated Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Outdated Show resolved Hide resolved
client/trino-cli/src/test/java/io/trino/cli/TestMarkdownTablePrinter.java Show resolved Hide resolved
@nineinchnick nineinchnick force-pushed the cli-markdown-output branch from 7b42f2d to 58d9ff9 Compare July 21, 2023 09:22
@nineinchnick nineinchnick force-pushed the cli-markdown-output branch from 58d9ff9 to 08a4348 Compare July 21, 2023 09:24
@nineinchnick nineinchnick force-pushed the cli-markdown-output branch from 08a4348 to 109762e Compare July 21, 2023 12:33
@nineinchnick
Copy link
Member Author

@wendigo PTAL

1 similar comment
@nineinchnick
Copy link
Member Author

@wendigo PTAL

@nineinchnick
Copy link
Member Author

@electrum @wendigo PTAL

@nineinchnick
Copy link
Member Author

@electrum @wendigo PTAL, it should be ready to go

@wendigo
Copy link
Contributor

wendigo commented Aug 16, 2023

@nineinchnick hi! i'm back from holidays, I'll review it soon :)

wendigo
wendigo approved these changes Aug 16, 2023
View reviewed changes
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java
private final List<Align> alignments;
private final Writer writer;

private boolean headerOutput;
Copy link
Contributor

Choose a reason for hiding this comment

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

bump

client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/main/java/io/trino/cli/MarkdownTablePrinter.java Show resolved Hide resolved
client/trino-cli/src/test/java/io/trino/cli/TestMarkdownTablePrinter.java Show resolved Hide resolved
@wendigo
Copy link
Contributor

wendigo commented Aug 23, 2023

We've discussed offline to follow-up with the refactoring of the code coming from/being similiar to aligned writer. LGTM.

@wendigo wendigo merged commit d30963d into trinodb:master Aug 23, 2023
@github-actions github-actions bot added this to the 425 milestone Aug 23, 2023
@colebow colebow mentioned this pull request Aug 23, 2023
Merged
@nineinchnick nineinchnick deleted the cli-markdown-output branch August 25, 2023 14:06
@nineinchnick nineinchnick mentioned this pull request Aug 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wendigo wendigo wendigo approved these changes

Assignees
No one assigned
Labels
cla-signed docs
Milestone
425
Development

Successfully merging this pull request may close these issues.
2 participants
@nineinchnick @wendigo