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

feat(ruby.cson) support quoted heredoc literals #212

Merged
merged 2 commits into from Oct 17, 2018

Conversation

Projects
None yet
4 participants
@rmosolgo
Copy link
Contributor

rmosolgo commented Aug 18, 2017

Thanks for maintaining this project and considering my PR!

Description of the Change

Ruby supports quoted heredocs:

  string1 = <<-STR
    Hello World
  STR

  string2 = <<-"STR"
    Hello World
  STR

  string3 = <<-'STR'
    Hello World
  STR

  shell_block = <<-`COMMAND`
    ls ~
    pwd
  COMMAND

But they aren't captured by the current grammar. This change adds quotes to the heredoc regexps so that they'll be properly tokenized.

Alternate Designs

No alternate deisgns were considered.

Benefits

We use single-quoted heredocs to assert that interpolation is not possible. This is a security check: interpolation can lead to various injection bugs.

Embedded code is not highlighted right now:

image

But after this change, it will be highlighted!

Possible Drawbacks

Added complexity to the heredoc regexp.

Applicable Issues

I didn't find any.

@rmosolgo

This comment has been minimized.

Copy link
Contributor Author

rmosolgo commented Sep 12, 2017

@50Wliu Does this concept sound ok to you?

@50Wliu

This comment has been minimized.

Copy link
Member

50Wliu commented Sep 12, 2017

I'll get around to it soon. Without looking at the code changes, it sounds like a good thing to add support for.

@rmosolgo

This comment has been minimized.

Copy link
Contributor Author

rmosolgo commented Sep 12, 2017

Thanks for taking a quick look, glad to hear I'm on the right track :)

@rmosolgo

This comment has been minimized.

Copy link
Contributor Author

rmosolgo commented Jun 27, 2018

👋 Hi, if someone has a chance, I'd appreciate a review of this! (or feel free to close it, if it's not a good fit for merging.)

@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Sep 25, 2018

@maxbrunsfeld Can you take a look here perhaps?

@asheren asheren referenced this pull request Oct 16, 2018

Closed

Iteration Plan: October 15 - October 26 #18237

0 of 15 tasks complete
@daviwil

This comment has been minimized.

Copy link
Member

daviwil commented Oct 17, 2018

Hi @rmosolgo, thanks a lot for the PR! The grammar changes look good to me, but do you mind adding an additional case to test backtick quoting? Once that's done this PR should be good to merge!

@daviwil daviwil self-assigned this Oct 17, 2018

@daviwil

This comment has been minimized.

Copy link
Member

daviwil commented Oct 17, 2018

Just realized I can easily just add the extra case through the web editor :) Once CI passes I'll get this merged and updated!

@daviwil daviwil merged commit f4082a0 into atom:master Oct 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rmosolgo

This comment has been minimized.

Copy link
Contributor Author

rmosolgo commented Oct 17, 2018

Woah, thanks! I'm looking forward to using this 😊

@rmosolgo rmosolgo deleted the rmosolgo:support-quoted-heredoc branch Oct 17, 2018

wingrunr21 added a commit to rubyide/vscode-ruby that referenced this pull request Dec 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.