only treat emphasis markdown based on spaces

RESOLVED FIXED

Status

()

Bugzilla
Creating/Changing Bugs
RESOLVED FIXED
3 years ago
9 months ago

People

(Reporter: Albert Ting, Assigned: gerv)

Tracking

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

3 years ago
Currently the empahsis markdown feature looks for underscores after a non-word.  Instead, it should be after a space or newline.

For example, this comment below:

   <prefix>_bar.c and <prefix>_bif.c

gets converted to below (with the underscore removed, and italicized)

   <prefix>bar.c and <prefix>bif.c
           |-- italicized --| 

I agree using code blocks could solve this, but it's not intuitive.
(Reporter)

Comment 1

2 years ago
Created attachment 8757065 [details] [diff] [review]
v1

I took a stab at fixing this.  The patch essentially changes the starting character from \W to \s.  This seems to work for me.
Attachment #8757065 - Flags: review?(dylan)
(Reporter)

Comment 2

2 years ago
Created attachment 8759222 [details]
101emphasis.t

I've attached the test case I'm using for this patch
Attachment #8759222 - Flags: review?(dylan)

Comment 3

2 years ago
We should have one single 100markdown.t test script, IMO. I'm really not a fan of having tons of markdown test scripts. For instance, we have one single 007util.t test script to test subroutines from Bugzilla::Util.
I agree, it should be in 100markdown.t.

Comment 5

2 years ago
Due to its complicated structure, the markdown is so vulnerable to further bugs when one bug gets fixed. Thus, I believe that there should be a separate bug addressing different test cases as a test script for markdown.
(Assignee)

Comment 6

a year ago
Created attachment 8784402 [details] [diff] [review]
Patch v.2

Here's a version with an updated 100markdown.t which reads .md files from a subdirectory and tests them. So adding new tests just means adding a new .md file to the subdirectory. The files are self-documenting - markdown above the "---", actual HTML below it. (This works because Markdown is allowed to contain HTML.) This means you can also view or process the .md file in other processors to see how they handle the same markup.

Gerv
Assignee: create-and-change → gerv
Attachment #8757065 - Attachment is obsolete: true
Attachment #8759222 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8757065 - Flags: review?(dylan)
Attachment #8759222 - Flags: review?(dylan)
Attachment #8784402 - Flags: review?(dylan)
I suspect I bit-rotted part of this with my fix in bug 1294569, but I wanted to say that this is a great improvement in testing. Thanks gerv!
(Assignee)

Comment 8

a year ago
dylan: you're welcome; feel free to give it r+ :-)

Gerv
(Assignee)

Comment 9

a year ago
Created attachment 8790326 [details] [diff] [review]
Patch v.3

Unbitrotted.

Gerv
Attachment #8784402 - Attachment is obsolete: true
Attachment #8784402 - Flags: review?(dylan)
Attachment #8790326 - Flags: review?(dylan)
(Assignee)

Comment 10

a year ago
dylan: any plans to review? :-)

Gerv
Comment on attachment 8790326 [details] [diff] [review]
Patch v.3

r=dylan
Attachment #8790326 - Flags: review?(dylan) → review+
Actually this used File::Slurp which only works because the jobqueue depends on it still, but I'll fix on commit.
To github.com:bugzilla/bugzilla.git
   4480c8ca9..d04520659  master -> master
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.