Skip to content

fix(bash): keep heredoc continuations highlighted#4415

Open
puneetdixit200 wants to merge 1 commit into
highlightjs:mainfrom
puneetdixit200:fix-bash-heredoc-continuation
Open

fix(bash): keep heredoc continuations highlighted#4415
puneetdixit200 wants to merge 1 commit into
highlightjs:mainfrom
puneetdixit200:fix-bash-heredoc-continuation

Conversation

@puneetdixit200

Copy link
Copy Markdown

Fixes #4377.

Summary

  • Keep Bash heredoc continuation commands on the command line instead of folding them into the heredoc string.
  • Start the heredoc string span on the following line and end it at the matching delimiter.
  • Add a Bash markup regression for <<-EOF | cat.
  • Update the existing Bash heredoc expectation and changelog entry.

Tests

  • Red before fix: the reported sample highlighted EOF | cat inside the heredoc string span, so cat was not highlighted as a command.
  • ONLY_LANGUAGES=bash npm run test-markup
  • npx eslint --no-eslintrc -c .eslintrc.lang.js src/languages/bash.js
  • npm run build
  • git diff --check

Note: I also ran the full npm run test-markup; it reached 537 passing tests and then hit the existing 2s Mocha timeout on http/default, unrelated to the Bash fixture. The Bash markup suite passed in both focused runs.

Comment thread src/languages/bash.js
Comment on lines +391 to +399
contains: [
ARITHMETIC,
COMMENT,
PATH_MODE,
QUOTE_STRING,
ESCAPED_QUOTE,
APOS_STRING,
ESCAPED_APOS,
VAR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All of this is because of the pipe and piping the string into any sequence of bash, right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not only pipes. The same split is needed for any shell syntax after the delimiter word, for example <<EOF >out or <<EOF && echo queued. The body should start at the following newline; everything before that newline should remain normal Bash.

Comment thread src/languages/bash.js
const HERE_DOC_BODY = {
className: 'string',
begin: /\n/,
end: /^\t*(\w+)$/m,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the requirement for heredoc naming in Bash? Could this be tightened up any further than \w+?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this needs broadening rather than tightening. Bash takes a word after << and applies quote removal; it is not limited to \w+.

For example, this branch captures only END here and never finds the real terminator, so echo done stays inside the string:

cat <<END-OF | cat
hello
END-OF
echo done

<<'EOF' is valid too, but is not recognized as a heredoc by this matcher. Could we add both as markup cases before settling on the delimiter match?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure. Add all the cases we know of, then try to get the highlighted correctly - that's the way to do it.

@arturict

Copy link
Copy Markdown
Contributor

Nice rust escaping coverage. Could you include minimal unit case for escaped quotes/backslashes in char literals to aid review traceability?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(shell/bash) Wrong syntax highlight after a heredoc, until the end of the line.

3 participants