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.

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

Use “squiggly” heredocs. #19600

Merged
merged 2 commits into from Oct 20, 2017
Merged

Conversation

ilovezfs
Copy link
Contributor

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Goes together with Homebrew/brew#3319.

CC @reitermarkus @MikeMcQuaid

@@ -20,7 +20,7 @@ def install
end

test do
(testpath/"voices.abc").write <<-EOF.undent
(testpath/"voices.abc").write <<~EOF
Copy link
Member

Choose a reason for hiding this comment

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

@ilovezfs If you're feeling really pedantic I'd change these to EOS too but feel free to 👎 that.

Copy link
Contributor Author

@ilovezfs ilovezfs Oct 18, 2017

Choose a reason for hiding this comment

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

The set of all heredoc delimiters used in core is
EOS
'EOS'
XML
XSL
EOS_CONFIG
'EOS_CONFIG'
HTML
EOF
'EOF'
EOPLIST
EOI
'EOS_SRC'
EOM
EOCNF
PYTHON
PROTO
PLODRC
TEST_SCRIPT
ERR

Are there others besides EOF we also no longer ❤️ ?

Copy link
Member

Choose a reason for hiding this comment

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

@ilovezfs Personally I hate all non-EOS ones 😁. I think 'EOS' has different behaviour, though, in terms of quoting etc. Will defer to you on them.

Copy link
Member

Choose a reason for hiding this comment

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

I'm in favour of <<~LANGUAGE where it makes sense, because TextMate supports nested syntax highlighting this way, and other editors may too. EOS for the remaining cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reitermarkus which of them are taking advantage of that?

EOS
'EOS'
XML
XSL
EOS_CONFIG
'EOS_CONFIG'
HTML
EOF
'EOF'
EOPLIST
EOI
'EOS_SRC'
EOM
EOCNF
PYTHON
PROTO
PLODRC
TEST_SCRIPT
ERR

I'm guessing XML, XSL, HTML, and PYTHON? And maybe PROTO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed an all-EOS commit 7a81b5a but can consider backing some of those out.

Also, what is the magic delimiter for C? I assume we don't want to go down that road given how many C code examples we have.

Personally I think just standardizing on EOS is probably the simplest thing to do at least in formulae.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm guessing XML, XSL, HTML, and PYTHON? And maybe PROTO?

and maybe EOPLIST?

Copy link
Member

Choose a reason for hiding this comment

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

Also, what is the magic delimiter for C?

Just C.

and maybe EOPLIST?

plist doesn't have its own, but XML works. (PLIST_XML works too, or anything like *_XML.)

Copy link
Member

Choose a reason for hiding this comment

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

Full list: HTML, XML, CSS, SQL, CPP, C, JS|JAVASCRIPT, JQUERY, SH|SHELL, LUA, RUBY.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reitermarkus for now I'd like to proceed with standardizing on EOS but we could consider supporting editor heredoc syntax highlighting at some point if that becomes a broadly supported feature in editors beyond TextMate. However, I think the value proposition is pretty low if the language has to be manually specified via the heredoc delimiter because it's currently something humans would need to police, as opposed to something that could be automatically enforced.

@DomT4
Copy link
Member

DomT4 commented Oct 18, 2017

Yikes. This is quite some diff.

@s172262
Copy link
Contributor

s172262 commented Oct 21, 2017

Test part ofgprof2dot causes error . gprof2dot needs to restore.
See #19128 (comment)

@ilovezfs
Copy link
Contributor Author

There's a form feed character on line 68 causing it.

@ilovezfs
Copy link
Contributor Author

@s172262 fixed in #19683.

@ilovezfs ilovezfs mentioned this pull request Oct 30, 2017
4 tasks
@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants