knrafto / language-bash

Parse and pretty-print Bash shell scripts
BSD 3-Clause "New" or "Revised" License
35 stars 9 forks source link

Here documents are rendered poorly #22

Closed mmhat closed 4 years ago

mmhat commented 4 years ago

Consider the following bash snippet:

cat <<EOF
here document
EOF

If one parses it with Language.Bash.Parse.parse and pretty prints it with Language.Bash.Pretty.prettyText it looks like that:

cat <<EOF
here document
EOF
;

Note the semicolon at the end: Bash will refuse the script with "line 4: syntax error near unexpected token `;'".

mmhat commented 4 years ago

I'd prefer the the following solution:

  1. Add a new ListTerm: SequentialNewline
  2. Use that one iff the last redirection of the last command in a statement is a here document

I could implement that if that's fine for you.

knrafto commented 4 years ago

Contributions are always welcome :)

Shouldn't the correct pretty-printing be this?

cat <<EOF
here document
EOF;

and maybe there's a separate improvement where pretty-printing a Sequential ListTerm should print a newline in most cases.

mmhat commented 4 years ago

No, the end-of-heredoc marker is required to be on a separate line, e.g. "EOF;" is not the end marker for "EOF". So it is good that there is a newline after the EOF but bash complains about the dangling semicolon.

I'm wondering if we should treat here documents entirely different: I don't think there can be more than one here document per command and it must be the last input redirection? If so, shouldn't it be Command :: ShellCommand -> [Redir] -> Maybe HereDoc -> Command? Or did I miss something?

knrafto commented 4 years ago

Okay, I played around with this a bit. I think the correct pretty-printing should be:

cat <<EOF ;
here document
EOF

since this works if the command ends with & as well.

It doesn't have to be the last redirection, for instance this works:

cat <<EOF >/tmp/heredoc.txt
here document
EOF

In fact you have have more than one heredoc:

cat <<EOF1 <<EOF2
here document 1
EOF1
here document 2
EOF2

although the second one overrides the first. It's still syntactically correct though.

So it seems like when pretty-printing heredocs, the correct thing would be to only print the "<<EOF" immediately and save the rest of it away for later. When we reach a newline, we dump the saved heredocs before continuing on. That might require some major changes to the pretty-printing machinery to hold the extra state but it's certainly doable.

knrafto commented 4 years ago

Btw, sorry that this has gone unnoticed for so long. My primary use case for this library was parsing, which is why the pretty-printing isn't that robust (there's no tests for it either).

mmhat commented 4 years ago

Hey, thank you very much! Apparently my bash-foo is not as strong as I thought :) I'll have a look at that and work something out.

mmhat commented 4 years ago

Fixed by #33. Closing.