Skip to content

Loading…

Fix #57 -- SmartyPants handling of single quotes. #200

Merged
merged 3 commits into from

5 participants

@mmorearty

git bisect indicates that the single quote bug was introduced in commit ad2c7fe. The problem is that by the time quotation marks are being converted, ' has already been changed into ', and the code to convert that into a smart quote is only looking for ', not &#39.

There was already code to handle the fact that " had been converted into ", but it appears that single-quotes were overlooked.

@mmorearty

Hang on please, there is a bug in this pull request that I need to fix...

@mmorearty

Okay, it's good to go

@aprescott

Your branch has a broken rake for me:

git clone https://github.com/mmorearty/redcarpet.git
cd redcarpet
git checkout patch-single-quotes
bundle install
rake
mkdir -p tmp/x86_64-linux/redcarpet/1.9.3
cd tmp/x86_64-linux/redcarpet/1.9.3
/home/adam/.rvm/rubies/ruby-1.9.3-p286/bin/ruby -I. ../../../../ext/redcarpet/extconf.rb
creating Makefile
cd -
cd tmp/x86_64-linux/redcarpet/1.9.3
make
compiling ../../../../ext/redcarpet/html_smartypants.c
../../../../ext/redcarpet/html_smartypants.c: In function ‘squote_len’:
../../../../ext/redcarpet/html_smartypants.c:94:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
../../../../ext/redcarpet/html_smartypants.c:94:2: note: use option -std=c99 or -std=gnu99 to compile your code
make: *** [html_smartypants.o] Error 1
rake aborted!
Command failed with status (2): [make...]
/home/adam/.rvm/gems/ruby-1.9.3-p286/gems/rake-compiler-0.8.1/lib/rake/extensiontask.rb:112:in `block (2 levels) in define_compile_tasks'
/home/adam/.rvm/gems/ruby-1.9.3-p286/gems/rake-compiler-0.8.1/lib/rake/extensiontask.rb:111:in `block in define_compile_tasks'
Tasks: TOP => default => test => test:unit => compile => compile:x86_64-linux => compile:redcarpet:x86_64-linux => copy:redcarpet:x86_64-linux:1.9.3 => tmp/x86_64-linux/redcarpet/1.9.3/redcarpet.so
(See full trace by running task with --trace)
@aprescott

Same error when trying to install the built gem:

Building native extensions.  This could take a while...
ERROR:  Error installing redcarpet-2.2.2.gem:
    ERROR: Failed to build gem native extension.

        /home/adam/.rvm/rubies/ruby-1.9.3-p286/bin/ruby extconf.rb
creating Makefile

make
compiling buffer.c
compiling rc_render.c
compiling html.c
compiling stack.c
compiling markdown.c
compiling rc_markdown.c
compiling autolink.c
compiling html_smartypants.c
html_smartypants.c: In function ‘squote_len’:
html_smartypants.c:94:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
html_smartypants.c:94:2: note: use option -std=c99 or -std=gnu99 to compile your code
make: *** [html_smartypants.o] Error 1


Gem files will remain installed in /home/adam/.rvm/gems/ruby-1.9.3-p286/gems/redcarpet-2.2.2 for inspection.
Results logged to /home/adam/.rvm/gems/ruby-1.9.3-p286/gems/redcarpet-2.2.2/ext/redcarpet/gem_make.out
@mmorearty

That's weird; I have no idea why I didn't hit that compile error sooner. Sorry; fixed.

@aprescott

Looks good. I built a 2.2.2.quotes version of the gem and installed it, looks like it solves the bug, at least, as well as handling ' even in HTML:

Redcarpet::Markdown.new(Redcarpet::Render::SmartyHTML).render("<p>testing's &#39;quote&#39;s and <code>code's code</code></p>")
#=> "<p>testing&rsquo;s &lsquo;quote&rsquo;s and <code>code's code</code></p>\n"
@robin850
Collaborator

@mmorearty : Could you rebase your commit please? :smile: I can't merge it automatically. If you want me to do it manually, let me know. It looks good to me. Thanks a lot!

@robin850 robin850 merged commit 0e97ce6 into vmg:master
@robin850
Collaborator

@mmorearty : Sorry it was a merge conflict, your commits were correctly squashed ; I allow myself to merge it locally. Thanks a lot for this contribution!

@mmorearty

Great, thanks @robin850 !

@mmorearty mmorearty deleted the mmorearty:patch-single-quotes branch
@parkr

This is really annoying for programmers, just a head's up:

jekyll serve --watch

If that were to turn into &emdash; watch, it would be incorrect. Is there a way to override this?

Enclose it in backticks.

Kewl.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 14, 2013
  1. @mmorearty

    Fix #57 -- SmartyPants handling of single quotes.

    mmorearty committed with Mike Morearty
  2. @mmorearty

    Correct output when single quote is NOT being converted

    mmorearty committed with Mike Morearty
    E.g. if the original text is "test'test".
Commits on Mar 14, 2013
  1. @mmorearty

    Fix compile error in C code

    mmorearty committed with Mike Morearty
Showing with 84 additions and 7 deletions.
  1. +2 −2 ext/redcarpet/buffer.h
  2. +54 −5 ext/redcarpet/html_smartypants.c
  3. +28 −0 test/redcarpet_test.rb
View
4 ext/redcarpet/buffer.h
@@ -44,7 +44,7 @@ struct buf {
size_t unit; /* reallocation unit size (0 = read-only buffer) */
};
-/* CONST_BUF: global buffer from a string litteral */
+/* CONST_BUF: global buffer from a string literal */
#define BUF_STATIC(string) \
{ (uint8_t *)string, sizeof string -1, sizeof string, 0, 0 }
@@ -52,7 +52,7 @@ struct buf {
#define BUF_VOLATILE(strname) \
{ (uint8_t *)strname, strlen(strname), 0, 0, 0 }
-/* BUFPUTSL: optimized bufputs of a string litteral */
+/* BUFPUTSL: optimized bufputs of a string literal */
#define BUFPUTSL(output, literal) \
bufput(output, literal, sizeof literal - 1)
View
59 ext/redcarpet/html_smartypants.c
@@ -83,6 +83,26 @@ word_boundary(uint8_t c)
return c == 0 || isspace(c) || ispunct(c);
}
+// If 'text' begins with any kind of single quote (e.g. "'" or "&apos;" etc.),
+// returns the length of the sequence of characters that makes up the single-
+// quote. Otherwise, returns zero.
+static size_t
+squote_len(const uint8_t *text, size_t size)
+{
+ static char* single_quote_list[] = { "'", "&#39;", "&#x27;", "&apos;", NULL };
+ char** p;
+
+ for (p = single_quote_list; *p; ++p) {
+ size_t len = strlen(*p);
+ if (size >= len && memcmp(text, *p, len) == 0) {
+ return len;
+ }
+ }
+
+ return 0;
+}
+
+// Converts " or ' at very beginning or end of a word to left or right quote
static int
smartypants_quotes(struct buf *ob, uint8_t previous_char, uint8_t next_char, uint8_t quote, int *is_open)
{
@@ -100,23 +120,33 @@ smartypants_quotes(struct buf *ob, uint8_t previous_char, uint8_t next_char, uin
return 1;
}
+// Converts ' to left or right single quote; but the initial ' might be in
+// different forms, e.g. &apos; or &#39; or &#x27;.
+// 'squote_text' points to the original single quote, and 'squote_size' is its length.
+// 'text' points at the last character of the single-quote, e.g. ' or ;
static size_t
-smartypants_cb__squote(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
+smartypants_squote(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size,
+ const uint8_t *squote_text, size_t squote_size)
{
if (size >= 2) {
uint8_t t1 = tolower(text[1]);
+ int next_squote_len = squote_len(text+1, size-1);
- if (t1 == '\'') {
- if (smartypants_quotes(ob, previous_char, size >= 3 ? text[2] : 0, 'd', &smrt->in_dquote))
- return 1;
+ // convert '' to &ldquo; or &rdquo;
+ if (next_squote_len > 0) {
+ uint8_t next_char = (size > 1+next_squote_len) ? text[1+next_squote_len] : 0;
+ if (smartypants_quotes(ob, previous_char, next_char, 'd', &smrt->in_dquote))
+ return next_squote_len;
}
+ // Tom's, isn't, I'm, I'd
if ((t1 == 's' || t1 == 't' || t1 == 'm' || t1 == 'd') &&
(size == 3 || word_boundary(text[2]))) {
BUFPUTSL(ob, "&rsquo;");
return 0;
}
+ // you're, you'll, you've
if (size >= 3) {
uint8_t t2 = tolower(text[2]);
@@ -133,10 +163,18 @@ smartypants_cb__squote(struct buf *ob, struct smartypants_data *smrt, uint8_t pr
if (smartypants_quotes(ob, previous_char, size > 0 ? text[1] : 0, 's', &smrt->in_squote))
return 0;
- bufputc(ob, text[0]);
+ bufput(ob, squote_text, squote_size);
return 0;
}
+// Converts ' to left or right single quote.
+static size_t
+smartypants_cb__squote(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
+{
+ return smartypants_squote(ob, smrt, previous_char, text, size, text, 1);
+}
+
+// Converts (c), (r), (tm)
static size_t
smartypants_cb__parens(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
{
@@ -164,6 +202,7 @@ smartypants_cb__parens(struct buf *ob, struct smartypants_data *smrt, uint8_t pr
return 0;
}
+// Converts "--" to em-dash, etc.
static size_t
smartypants_cb__dash(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
{
@@ -181,6 +220,7 @@ smartypants_cb__dash(struct buf *ob, struct smartypants_data *smrt, uint8_t prev
return 0;
}
+// Converts &quot; etc.
static size_t
smartypants_cb__amp(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
{
@@ -189,6 +229,11 @@ smartypants_cb__amp(struct buf *ob, struct smartypants_data *smrt, uint8_t previ
return 5;
}
+ int len = squote_len(text, size);
+ if (len > 0) {
+ return (len-1) + smartypants_squote(ob, smrt, previous_char, text+(len-1), size-(len-1), text, len);
+ }
+
if (size >= 4 && memcmp(text, "&#0;", 4) == 0)
return 3;
@@ -196,6 +241,7 @@ smartypants_cb__amp(struct buf *ob, struct smartypants_data *smrt, uint8_t previ
return 0;
}
+// Converts "..." to ellipsis
static size_t
smartypants_cb__period(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
{
@@ -213,6 +259,7 @@ smartypants_cb__period(struct buf *ob, struct smartypants_data *smrt, uint8_t pr
return 0;
}
+// Converts `` to opening double quote
static size_t
smartypants_cb__backtick(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
{
@@ -224,6 +271,7 @@ smartypants_cb__backtick(struct buf *ob, struct smartypants_data *smrt, uint8_t
return 0;
}
+// Converts 1/2, 1/4, 3/4
static size_t
smartypants_cb__number(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
{
@@ -256,6 +304,7 @@ smartypants_cb__number(struct buf *ob, struct smartypants_data *smrt, uint8_t pr
return 0;
}
+// Converts " to left or right double quote
static size_t
smartypants_cb__dquote(struct buf *ob, struct smartypants_data *smrt, uint8_t previous_char, const uint8_t *text, size_t size)
{
View
28 test/redcarpet_test.rb
@@ -52,6 +52,34 @@ def test_that_smart_gives_d_suffix_a_rsquo
end
end
+class SmartyHTMLTest < Test::Unit::TestCase
+ def setup
+ @smarty_markdown = Redcarpet::Markdown.new(Redcarpet::Render::SmartyHTML)
+ end
+
+ def test_that_smartyhtml_converts_single_quotes
+ markdown = @smarty_markdown.render("They're not for sale.")
+ assert_equal "<p>They&rsquo;re not for sale.</p>\n", markdown
+ end
+
+ def test_that_smartyhtml_converts_double_quotes
+ rd = @smarty_markdown.render(%("Quoted text"))
+ assert_equal %(<p>&ldquo;Quoted text&rdquo;</p>\n), rd
+ end
+
+ def test_that_smartyhtml_ignores_pre
+ rd = @smarty_markdown.render(" It's a test of \"pre\"\n")
+ expected = "It&#39;s a test of &quot;pre&quot;"
+ assert rd.include?(expected), "\"#{rd}\" should contain \"#{expected}\""
+ end
+
+ def test_that_smartyhtml_ignores_code
+ rd = @smarty_markdown.render("`It's a test of \"code\"`\n")
+ expected = "It&#39;s a test of &quot;code&quot;"
+ assert rd.include?(expected), "\"#{rd}\" should contain \"#{expected}\""
+ end
+end
+
class HTMLRenderTest < Test::Unit::TestCase
def setup
@markdown = Redcarpet::Markdown.new(Redcarpet::Render::HTML)
Something went wrong with that request. Please try again.