Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki - Mailing list pgsql-hackers

From Jan Michálek
Subject Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki
Date
Msg-id CAAYBy8aZJnQ2pFVGHmKO4q=d_J5+JNkqw6vPcxG6xEQtdHBhUA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki  (Pierre Ducroquet <p.psql@pinaraf.info>)
List pgsql-hackers


2017-03-23 17:26 GMT+01:00 Pierre Ducroquet <p.psql@pinaraf.info>:
The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

Hi

This is my first review (Magnus said in his presentation in PGDay Paris that volunteers should just come and help, so here I am), so please notify me for any mistake I do when using the review tools...

The feature seems to work as expected, but I don't claim to be a markdown and rst expert.
Some minor issues with the code itself :
- some indentation issues (documentation and code itself with mix between space based and tab based indentation) and a few trailing spaces in code
- typographic issues in the documentation :
  - "The html, asciidoc, latex, latex-longtable, troff-ms, and markdown and rst formats" ==> duplicated and
  - "Sets the output format to one of unaligned, aligned, wrapped, html, asciidoc, latex (uses tabular), latex-longtable, rst, markdown, or troff-ms." ==> extra comma at the end of the list
- the comment " dont add line after last row, because line is added after every row" is misleading, it should warn that it's only for rst
- there is a block of commented out code left
- in the print_aligned_vertical function, there is a mix between "cont->opt->format == PRINT_RST" and "format == &pg_rst" and I don't see any obvious reason for that
- the documentation doesn't mention (but ok, it's kind of obvious) that the linestyle option will not work with rst and markdown

Thanks !

Thanks
I will work on it this weekend. I need to adapt it to current master and i will do some indentation issues with this.
I need to add \x to markdown format and some things about title from older posts there.

Nice Day Je;

 

The new status of this patch is: Waiting on Author

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



--
Jelen
Starší čeledín datovýho chlíva

pgsql-hackers by date:

Previous
From: Teodor Sigaev
Date:
Subject: Re: [HACKERS] Potential data loss of 2PC files
Next
From: Teodor Sigaev
Date:
Subject: Re: [HACKERS] Re: [BUGS] Problem in using pgbench's --connect(-C) and--rate=rate(-R rate) options together.