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

Language name with dash #246

Closed
wants to merge 2 commits into from
Closed

Language name with dash #246

wants to merge 2 commits into from

Conversation

@maxmeyer
Copy link
Contributor

@maxmeyer maxmeyer commented Apr 19, 2015

@gettalong

A new one for #244. This line might be worth to mention. Without the addition latex is not able to compile the document. If you've got a better idea where I should place the code, I'm open for suggestions.

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 20, 2015

  • The regexp should be \w[\w-]* so that it does not start with a dash.
  • Please remove the tmp/ directory from .gitignore.
  • You need to add the test to the EXCLUDE_LATEX_FILES list to avoid running the conversion instead of the added code.
@maxmeyer
Copy link
Contributor Author

@maxmeyer maxmeyer commented Apr 20, 2015

Fixed. But now some coderay tests are failing. Any idea how to fix that or is there another EXCLUDE_ I can use for this?

@maxmeyer
Copy link
Contributor Author

@maxmeyer maxmeyer commented Apr 20, 2015

Ok. Now I understand your test-setup better. I added an options-file as well. Should I add a test for coderay as well?

42
end
~~~


This comment has been minimized.

@gettalong

gettalong Apr 20, 2015
Owner

Please only add an example that uses a valid, real world language. Otherwise people might think that ruby-trunk is some kind of Ruby based language.

Personally, I would not add any example because we never explicitly state the syntax requirements for the language. Also if a user wants to highlight a programming language that contains a dash he will just use it in the fenced code blocks.

This comment has been minimized.

@maxmeyer

maxmeyer Apr 20, 2015
Author Contributor

Fixed.

@@ -27,7 +27,7 @@ def self.call(converter, text, lang, type, _unused_opts)
if type == :span && lang
::CodeRay.scan(text, lang.to_sym).html(options(converter, :span)).chomp
elsif type == :block && (lang || options(converter, :default_lang))
lang = (lang || options(converter, :default_lang)).to_sym
lang = (lang || options(converter, :default_lang)).to_s.gsub(/-/, '_').sub(/language_/, 'language-').to_sym
::CodeRay.scan(text, lang).html(options(converter, :block)).chomp << "\n"

This comment has been minimized.

@gettalong

gettalong Apr 20, 2015
Owner

The first gsub I understand but why the second sub?

This comment has been minimized.

@maxmeyer

maxmeyer Apr 20, 2015
Author Contributor

Fixed.

42
end
~~~

This comment has been minimized.

@gettalong

gettalong Apr 20, 2015
Owner

Why all the tests below? They don't make sense for this feature enhancement.

This comment has been minimized.

@maxmeyer

maxmeyer Apr 20, 2015
Author Contributor

To make sure, that a language with a dash behaves the same like a language without a dash

This comment has been minimized.

@maxmeyer

maxmeyer Apr 20, 2015
Author Contributor

Removed.

@@ -0,0 +1,2 @@
:enable_coderay: false

This comment has been minimized.

@gettalong

gettalong Apr 20, 2015
Owner

I think there shouldn't be any need for this file anymore since you modify the language name for coderay to use underscores instead of dashes.

This comment has been minimized.

@maxmeyer

maxmeyer Apr 20, 2015
Author Contributor

I added this because the HTML generated without the coderay converter is much simpler and is the same like for the existing tests.

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 20, 2015

Thanks for everything and your patience! The changes are now in master.

@maxmeyer
Copy link
Contributor Author

@maxmeyer maxmeyer commented Apr 20, 2015

Wonderful. 😄 Thanks a lot. Any plans for a new release?

@gettalong
Copy link
Owner

@gettalong gettalong commented Apr 20, 2015

Probably end of this week.

@maxmeyer
Copy link
Contributor Author

@maxmeyer maxmeyer commented Apr 20, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants