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

Add R Markdown support (part deux) #93

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

burodepeper
Copy link
Owner

@burodepeper burodepeper commented Feb 16, 2016

  • Add better/wider support for using functions as attribute values (ie, {r code=capture.output(dump('fivenum', ''))})
  • Use value of engine attribute to parse the contents of the code-block
  • Bug: trailing space shouldn't be needed, ie: {r }

@ZigZagT
Copy link

ZigZagT commented Oct 9, 2016

The tailing space issue, I found it's more serious than it looks. See the two screenshots:

without tailing space:
screen shot 2016-10-09 at 21 21 26

with tailing space at line 2:
screen shot 2016-10-09 at 21 21 34

@burodepeper
Copy link
Owner Author

I can't tell for sure from your minimap screenshots, but my first impression is (because that has been the case more often) that the fenced-code-block doesn't close properly until it finds a next ``` instead of the one you intended. If you feel comfortable with it, I invite you to try your hand at a fix. The code that's causing this can be found here.

@ZigZagT
Copy link

ZigZagT commented Oct 9, 2016

This should be more clear.
It seems it not recognizes the whole {js}</code> pattern, until a tailing space is added. However, the <code>{r} pattern don't met this issue.

screen shot 2016-10-09 at 21 38 11

screen shot 2016-10-09 at 21 36 01

I'll try to fix it

@burodepeper
Copy link
Owner Author

Yeah, that's what I meant. In your second example it detects the start of the code-block on line 10, but it doesn't detect the closing element until line 18. Let me know if you figure it out, and feel free to PR any changes into this dev-r-markdown branch.

@ZigZagT
Copy link

ZigZagT commented Oct 9, 2016

#139 fix it

@kylebarron
Copy link
Contributor

The trailing space is trivial to fix.

With the current compiled grammar, i.e. for js:

"begin": "^\\s*([`~]{3,})\\s*(\\{?)((?:\\.?)(?:javascript|js|jsx))(?=( |$|{))\\s*(\\{?)([^`\\{\\}]*)(\\}?)$",

The regex includes the group (?=( |$|{)) after the language name, which forces a space (or $ or { after the language name. Changing this to e.g.:

(?=(}| |$|{))

fixes the trailing space problem.

@kylebarron
Copy link
Contributor

r without the trailing space always worked because the grammars/repositories/flavors/rmarkdown.cson file didn't require a space after {r} and that file was included before the general code.

kylebarron pushed a commit to kylebarron/language-markdown that referenced this pull request Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants