Reddit slash links #32

jimmyjxiao · 2018-01-08T03:11:39Z

This implements an option for Reddit subreddit and user links such as /r/test or r/test. Sorry about my complete lack of skill with git

Links of form u/a are still broken (leading '/' in href is missing).

codecov · 2018-01-08T03:14:36Z

Codecov Report

Merging #32 into master will decrease coverage by 0.03%.
The diff coverage is 91.3%.

@@            Coverage Diff             @@
##           master      #32      +/-   ##
==========================================
- Coverage   92.75%   92.71%   -0.04%     
==========================================
  Files           1        1              
  Lines        2525     2567      +42     
==========================================
+ Hits         2342     2380      +38     
- Misses        183      187       +4

Impacted Files	Coverage Δ
md4c/md4c.c	`92.71% <91.3%> (-0.04%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0002e2...c959ac7. Read the comment docs.

mity

Thanks for working on this.

But I miss one important thing before considering merging this, and that is adding some tests. Please add some tests covering the new feature into a new file under test/[something].txt and update scripts/run-tests.sh accordingly.

mity · 2018-01-08T07:29:15Z

md2html/md2html.c

@@ -288,6 +293,7 @@ cmdline_callback(int opt, char const* value, void* data)

        case 'c':   parser_flags = MD_DIALECT_COMMONMARK; break;
        case 'g':   parser_flags = MD_DIALECT_GITHUB; break;
+        case 'R':   parser_flags = MD_DIALECT_REDDITPOST; break;



It's a minor detail, but please swap 'r' and 'R' so that there is some consistency: Lower case for dialect options, upper case for extensions.

mity · 2018-01-08T07:29:49Z

md2html/md2html.c

@@ -302,6 +308,7 @@ cmdline_callback(int opt, char const* value, void* data)
        case 'V':   parser_flags |= MD_FLAG_PERMISSIVEAUTOLINKS; break;
        case 'T':   parser_flags |= MD_FLAG_TABLES; break;
        case 'S':   parser_flags |= MD_FLAG_STRIKETHROUGH; break;
+        case 'r':   parser_flags |= MD_FLAG_REDDITSLASHDETECTION;


Missing break; here, isn't it?

mity · 2018-01-08T07:37:15Z

md4c/md4c.c

-                    break;
-                }
-
+				{


Can you remove the bogus blank lines in the code below?

mity · 2018-01-08T07:40:11Z

README.md

@@ -104,6 +104,8 @@ some extensions or allowing some deviations from the specification.

 * With the flag `MD_FLAG_NOINDENTEDCODEBLOCKS`, indented code blocks are
   disabled.
+
+ * With the flag `MD_FLAG_REDDITSLASHDETECTION`, Reddit subreddit and user links such as r/test or /u/me are detected


Please rename the flag for consistency with other flags. No other flag uses the word "detection". I am not reddit user, so IDK whether something like MD_FLAG_REDDITAUTOLINKS but it could give you an idea.

mity · 2018-01-08T07:41:56Z

md4c/md4c.c

@@ -2696,7 +2696,8 @@ md_build_mark_char_map(MD_CTX* ctx)
    ctx->mark_char_map['!'] = 1;
    ctx->mark_char_map[']'] = 1;
    ctx->mark_char_map['\0'] = 1;
-
+	if (ctx->r.flags & MD_FLAG_REDDITSLASHDETECTION)


Uff, indentation using tab chars here and on some other places of your patch. Please use just spaces as in the rest of the source file(s).

mity · 2018-01-08T07:50:45Z

md4c/md4c.h

@@ -271,6 +279,8 @@ typedef struct MD_SPAN_IMG_DETAIL {

 #define MD_FLAG_PERMISSIVEAUTOLINKS         (MD_FLAG_PERMISSIVEEMAILAUTOLINKS | MD_FLAG_PERMISSIVEURLAUTOLINKS | MD_FLAG_PERMISSIVEWWWAUTOLINKS)
 #define MD_FLAG_NOHTML                      (MD_FLAG_NOHTMLBLOCKS | MD_FLAG_NOHTMLSPANS)
+#define MD_FLAG_REDDITSLASHDETECTION             0x8000  /* Enable Reddit autolinks */
+#define MD_FLAG_REDDIT_SLASHES_AS_LINKS        0x4000 //Instead of making Reddit links into special spans, make them into web links


I think that the logic related to the MD_FLAG_REDDIT_SLASHES_AS_LINKS has nothing to do with the parsing but only with output rendering, and therefore it would be IMHO better to place the related logic just into the md2html utility. Take a look how it treats e.g. MD_RENDER_FLAG_VERBATIM_ENTITIES.

Or is there something what I have overlooked?

jimmyjxiao · 2018-01-09T23:20:55Z

I think I've fixed most of your concerns, can you take another look?

mity · 2018-01-10T08:25:30Z

I think I've fixed most of your concerns, can you take another look?

There is yet the problem with the blank lines between lines 4010 and 4045 in the file md4c.c which were introduced somewhere in your branch so please fix that too.

I will yet do some tests which can take few days before merging it, but so far it seems in quite a good shape to me.

mity · 2018-01-10T08:33:16Z

There is yet another problem: The new tests did not run correctly. See https://travis-ci.org/mity/md4c/builds/326630844?utm_source=github_status&utm_medium=notification (you may need to open the triangle mark on line 553 of the log to see the output of the tests).

EDIT: The culprit is in scripts/run-tests.sh:

$PYTHON "$TEST_DIR/spec_tests.py" -s
"$TEST_DIR/reddit-autolinks.txt" -p "$PROGRAM --freddit-autolinks"

The end-of-line generally means end of command, so shell sees this as two commands, not one with more options. So concatenating the two lines should solve it.

jimmyjxiao · 2018-01-11T23:49:40Z

Alright, should be fixed

DemiMarie and others added 6 commits August 5, 2017 23:16

Add mostly-complete support for Reddit-style autolinks

d210957

Links of form u/a are still broken (leading '/' in href is missing).

Finish Reddit autolinks

4a8f087

Merge remote-tracking branch 'ral/reddit' into Reddit-Slash-Links

e9295c0

Fully working Reddit slash links

19e6c59

Update md2html.c

c686873

undo this because it breaks tests

0129aa5

jimmyjxiao and others added 2 commits January 7, 2018 20:25

Update README.md

0d5d495

fix some comments:

fda1114

mity requested changes Jan 8, 2018

View reviewed changes

jimmyjxiao added 7 commits January 8, 2018 16:33

change Reddit flag name

93e9afd

Remove Reddit autolinks as web links option from md4c

6bd03e7

untabify

4486040

rename Reddit dialect option to ensure consistency

f1e5e0e

change md2html flags

aff66bf

add reddit autolinking to md2html and add tests

5233d98

oops forgot to add some stuff to the last commit

70e8801

jimmyjxiao added 2 commits January 11, 2018 16:40

remove extra linebreak in tests

061d6f6

remove extra linebreaks

3fb5f03

Update README.md

c959ac7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reddit slash links #32

Reddit slash links #32

jimmyjxiao commented Jan 8, 2018 •

edited

codecov bot commented Jan 8, 2018 •

edited

mity left a comment

mity Jan 8, 2018

mity Jan 8, 2018

mity Jan 8, 2018

mity Jan 8, 2018

mity Jan 8, 2018

mity Jan 8, 2018

jimmyjxiao commented Jan 9, 2018

mity commented Jan 10, 2018

mity commented Jan 10, 2018 •

edited

jimmyjxiao commented Jan 11, 2018

-                                  break;
-                              }
+              				{

Reddit slash links #32

Are you sure you want to change the base?

Reddit slash links #32

Conversation

jimmyjxiao commented Jan 8, 2018 • edited

codecov bot commented Jan 8, 2018 • edited

Codecov Report

mity left a comment

Choose a reason for hiding this comment

mity Jan 8, 2018

Choose a reason for hiding this comment

mity Jan 8, 2018

Choose a reason for hiding this comment

mity Jan 8, 2018

Choose a reason for hiding this comment

mity Jan 8, 2018

Choose a reason for hiding this comment

mity Jan 8, 2018

Choose a reason for hiding this comment

mity Jan 8, 2018

Choose a reason for hiding this comment

jimmyjxiao commented Jan 9, 2018

mity commented Jan 10, 2018

mity commented Jan 10, 2018 • edited

jimmyjxiao commented Jan 11, 2018

jimmyjxiao commented Jan 8, 2018 •

edited

codecov bot commented Jan 8, 2018 •

edited

mity commented Jan 10, 2018 •

edited