search

thomasrussellmurphy / istyle-verilog-formatter

Open source implementation of a Verilog formatter
GNU General Public License v2.0
174 stars 45 forks source link

Fix banner written to stdout in stdin->stdout mode #13

Closed lassik closed 5 years ago

lassik commented 5 years ago

Spoke too soon in PR #12. Despite having help in its name, printHelpTitle() actually prints the program's title even when help messages are not displayed. Since the title is printed to stdout it would get mixed up with code that is also written there from stdin.

Fix by changing the name of printHelpTitle() to the more obvious printTitle(), and by only calling it when formatting a list of one or more named files (in which case no code is written to stdout so it doesn't hurt to write the title there). In the case of stdin->stdout, no title is printed (as is customary for that use case).

Also, the first commit fixes something that is almost certainly a bug (missing exit() call).

lassik commented 5 years ago

Anyway, I tested this build by incorporating it into our Emacs formatting framework so it works as expected now. Also checked that is works when giving a filename on the command line.

thomasrussellmurphy commented 5 years ago

Does 790dcbcf0bba66b26c0b966fceceb2352e0e99b1 address your desired changes?

lassik commented 5 years ago

Looks perfect. Thank you for the quick work! I've never had such a fast response from the maintainer of any formatter :)

lassik commented 5 years ago

You can run all the same tests I did by having an example.v Verilog file, then

iStyle example.v(in-place formatting with filename argument) and

iStyle <example.v >example.out.v (stdin)

The stdin mode should now print nothing at all to stdout/stderr. The in-place mode prints the title (banner) as before.

lassik commented 5 years ago

In Emacs (and other editors) the stdin and stdout are Unix/Windows pipes that read/write editor buffers, but otherwise there is no difference. Code formatting algorithms are not typically sensitive to whether they are using files or pipes. You can't seek in a pipe, but formatters shouldn't be written to seek the stream anyway (they should only read forward, buffering everything they need using standard library functions).

lassik commented 5 years ago

Since you made equivalent changes, feel free to close this PR.

thomasrussellmurphy commented 5 years ago

Closed by 790dcbcf0bba66b26c0b966fceceb2352e0e99b1