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

Update lexer development guide #1145

Merged
merged 6 commits into from
Jun 21, 2019

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Jun 1, 2019

As per the suggestions from @jneen in #1111, this updates the lexer development guide.

As per the suggestions from @jneen in rouge-ruby#1111, this updates the lexer
development guide.
@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Jun 1, 2019
Copy link
Contributor

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

Some minor corrections needed..

docs/LexerDevelopment.md Outdated Show resolved Hide resolved
docs/LexerDevelopment.md Outdated Show resolved Hide resolved
docs/LexerDevelopment.md Outdated Show resolved Hide resolved
docs/LexerDevelopment.md Outdated Show resolved Hide resolved
pyrmont and others added 4 commits June 3, 2019 18:38
Co-Authored-By: Ashwin Maroli <ashmaroli@users.noreply.github.com>
Co-Authored-By: Ashwin Maroli <ashmaroli@users.noreply.github.com>
Co-Authored-By: Ashwin Maroli <ashmaroli@users.noreply.github.com>
@pyrmont
Copy link
Contributor Author

pyrmont commented Jun 3, 2019

Thanks @ashmaroli :) Suggestions incorporated!

@pyrmont
Copy link
Contributor Author

pyrmont commented Jun 16, 2019

In #1089, @MaximeKjaer pointed out that the 'Special Words' section doesn't use a valid reference to the memoised method.

@ashmaroli
Copy link
Contributor

@pyrmont Please clarify your statement above a bit further..

@jneen
Copy link
Member

jneen commented Jun 19, 2019

The "special words" section is also wrong - we generally discourage interpolating into regular expressions like that.

@jneen
Copy link
Member

jneen commented Jun 19, 2019

(the example is also valid - the rule call is done from a static context, so keywords resolves to the class method.)

@pyrmont
Copy link
Contributor Author

pyrmont commented Jun 19, 2019

Thanks @jneen :) Will update and push a new commit.

@pyrmont
Copy link
Contributor Author

pyrmont commented Jun 19, 2019

@ashmaroli Oh, and for context, this was the discussion to which I was referring where the reference is discussed.

@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Jun 21, 2019
@pyrmont pyrmont merged commit df73f3a into rouge-ruby:master Jun 21, 2019
@pyrmont pyrmont removed the author-action The PR has been reviewed but action by the author is needed label Jun 21, 2019

To see your visual sample, launch Rouge's visual test app by running `bundle
exec rackup`. You can choose your sample from the complete list by going to
<http://localhost:9292>. ot have a file extension. To start the test suite, run
Copy link
Contributor

Choose a reason for hiding this comment

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

Great document ! It looks like there is a missing piece of thee sentence here. I'd be happy to send a pull request to fix this but I don't know what you wanted to write.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hahahaha, you've got me—I'm not sure! I think this is an errant copy and paste and everything after 'ot' in this paragraph should be deleted.

@pyrmont pyrmont deleted the docs.update-lexer-guide branch January 8, 2020 20:07
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.

4 participants