9
\$\begingroup\$

I've made a simple HTML→Markdown converter in Javascript and am looking for any feedback. For now, I've basically used Stack Exchange's /editing-help as a guide as to what to convert, but I might look at CommonMark's spec later on.

It uses DOMParser() and then goes through the child nodes to convert things.

My test HTML string right now is:

<h1>h1</h1> 
<br>
<h2>h2</h2> 
<br>
<h3>h3</h3> 
<br>text outside everything
<br>
<h2>(and another element!)</h2> 
<br>
<img src='http://example.com/example.png'>
<br><a href='http://google.com'>a link!</a> 
<br>
<ul>
    <li>item 1</li>
    <li>item 2</li>
    <li>item 3</li>
</ul>
<br>
<ol>
    <li>item 1</li>
    <li>item 2</li>
    <li>item 3</li>
</ol>
<br><strong>BOLD TEXT</strong> and <i>ITALICISED TEXT</i> 
<br>
<blockquote>blockquote</blockquote>
<br>

and that conversion 'works':

# h1 

## h2 

### h3 

text outside everything 

## (and another element!) 

![enter image description here](http://example.com/example.png) 

[a link!](http://google.com) 

- item 1
- item 2
- item 3


1. item 1
2. item 2
3. item 3


**BOLD TEXT** and *ITALICISED TEXT* 

> blockquote

Code

var str = "<h1>h1</h1> <br>"
str += "<h2>h2</h2> <br>";
str += "<h3>h3</h3> <br>";
str += "text outside everything <br>";
str += "<h2>(and another element!)</h2> <br>"
str += "<img src='http://example.com/example.png'> <br>";
str += "<a href='http://google.com'>a link!</a> <br>";
str += "<ul><li>item 1</li><li>item 2</li><li>item 3</li></ul> <br>";
str += "<ol><li>item 1</li><li>item 2</li><li>item 3</li></ol> <br>";
str += "<strong>BOLD TEXT</strong> and <i>ITALICISED TEXT</i> <br>";
str += "<blockquote>blockquote</blockquote>";

var doc = new DOMParser().parseFromString(str, 'text/html');
var childnodes = doc.body.childNodes;
var markdown = '';

var conversions = {
  br: function(data) {
    return '\n\n';
  },
  h1: function(data) {
    return '# '
  },
  h2: function(data) {
    return '## ';
  },
  h3: function(data) {
    return '### ';
  },
  hr: function(data) {
    return '---\n';
  },
  blockquote: function(data) {
    return '> ';
  },
  img: function(data) {
    var imgStr = "![alt text](" + data.curEl.src + ")";
    return imgStr;
  },
  a: function(data) {
    return "[" + data.html + "](" + data.curEl.getAttribute('href') + ")";
  },
  ul: function(data) {
    var lis = childnodes[data.i].childNodes;
    var newmd = '';
    var lislength = lis.length;
    for (var x = 0; x < lislength; x++) {
      newmd += "- " + lis[x].innerHTML + "\n";
    }
    return newmd;
  },
  ol: function(data) {
    var lis = childnodes[data.i].childNodes;
    var counter = 1;
    var newmd = '';
    var lislength = lis.length
    for (var x = 0; x < lislength; x++) {
      newmd += counter + ". " + lis[x].innerHTML + "\n";
      counter++;
    }
    return newmd;
  },
  strong: function(data) {
    return "**" + data.html + "**";
  },
  i: function(data) {
    return "*" + data.html + "*";
  }
};


function convertToMarkdown(curEl, html, tag, i) {
  if (tag == undefined) { //for text nodes
    markdown += curEl.textContent;
  } else {
    tag = tag.toLowerCase();
    console.log(tag);
    markdown += conversions[tag]({
      curEl: curEl,
      html: html,
      tag: tag,
      i: i
    }) + (['ul', 'ol', 'i', 'strong', 'a'].indexOf(tag) > -1 ? '' : html);
  }
}

var length = childnodes.length;
for (var i = 0; i < length; i++) {
  var curEl = childnodes[i],
    html = childnodes[i].innerHTML,
    tag = childnodes[i].tagName;
  convertToMarkdown(curEl, html, tag, i);
}
console.log(markdown);

(you can check the output yourself in the console)

Main Questions:

  • Is my code readable? How can I make it more so?
  • Is there a cleaner way to do any part of this?
\$\endgroup\$
10
  • \$\begingroup\$ BTW I'm aware there are many converters that already exist - but I wanted to have a go myself... \$\endgroup\$ Aug 24, 2015 at 12:44
  • \$\begingroup\$ Your code fails on <ol> starting at values other than 1. Also, <ul> tags have a space before the - \$\endgroup\$ Aug 24, 2015 at 13:15
  • \$\begingroup\$ @IsmaelMiguel sorry, what do you mean? \$\endgroup\$ Aug 24, 2015 at 13:57
  • \$\begingroup\$ I mean that the counter is always starting at 1, when the starting number can be, for example, 10. \$\endgroup\$ Aug 24, 2015 at 14:15
  • 1
    \$\begingroup\$ Alright. That really isn't a problem then \$\endgroup\$ Aug 24, 2015 at 14:18

3 Answers 3

4
\$\begingroup\$

Your code isn't that much re-usable. I can't include it in a file and call it since I could easily break it from anywhere.

What I suggest is that you wrap it in the following block:

(function(window, undefined){
    'use strict';
    [code]
})(Function('return this')());

This will protect your code in so many ways:

  • You guarantee that the window object is the window
  • You guarantee that undefined is really undefined
  • You can be 100% sure that there won't be variables on the global scope
  • All variables are local (looks the same, but it is a tiny bit different)
  • The possibility of a script screwing up your code is null.

And this takes what? 1 minute? And you have to change almost nothing!


You will notice the 'use strict;' there. You may be alienated by it.

This has been re-re-re-re-re-re-itterated here. Using it adds some security features and prevent some 'stupid' bloopers and other mistakes and distractions.

You can read about it in greater detail on MDN's page.


You currently are writting everything into the markdown variable. Before going any further, let me take the liberty to teach you about the amazing return statement:

When a return statement is called in a function, the execution of this function is stopped. If specified, a given value is returned to the function caller. If the expression is omitted, undefined is returned instead.

Source: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/return

Inside your convertToMarkdown() function, instead of cramming it all into that variable, use a return.


Inside the same function, you have the following:

if (tag == undefined) { //for text nodes
    markdown += curEl.textContent;
} else {

Which isn't fine. It is too complex to explain in less than 20k characters, but that piece must be gone! I'll show how later on.

One of the problems with this is also that you are using .textContent. That isn't much compatible with other "browsers" (*cough* IE *cough*). The most compatible method is by using .innerText. But Firefox now decided that it wouldn't support it. Fear not, just add this line at the top:

var TEXT_PROPERTY = document.head.innerText ? 'innerText' : 'textContent';

And use it like curEl[TEXT_PROPERTY] to get the text content.


Instead of re-creating a parser object, you can simply create a <div>, keep it in memory, and use it to extract the elements. Setting the .innerHTML of that element will have the same effect than the parser, but faster. You then have access to tons of tools readily available on DOM elements.


Your choice of newlines is quite weird. Browsers will all the time convert them to \r\n. You can use it, since you know what to expect. Imagine what would be to write \n and get \r\n instead. That would drive some guys insane!


On your conversions object, you have methods that have 1 argument, but use it for nothing. You could just drop that useless argument. Done.


And now, my promised alternative:

(function(window, undefined){
    'use strict';
    var TEXT_PROPERTY = document.head.innerText ? 'innerText' : 'textContent';
    var NEWLINE = '\r\n';

    var indent = function(text){
        return text.replace(/(\r\n|\r|\n)/g,'$1    ');
    };

    var conversions = {
        BR: function(){
            return '  ' + NEWLINE;
        },
        P: function(elem) {
            return NEWLINE + NEWLINE + elem.innerHTML + NEWLINE;
        },
        H1: function(elem) {
            return '# ' + elem.innerHTML + NEWLINE;
        },
        H2: function(elem) {
            return '## ' + elem.innerHTML + NEWLINE;
        },
        H3: function(elem) {
            return '### ' + elem.innerHTML + NEWLINE;
        },
        HR: function() {
            //we use 4 to do not cause confusion with <strike></strike>
            return '----' + NEWLINE;
        },
        BLOCKQUOTE: function(elem) {
            return '> ' + elem
                    .innerHTML
                    .replace(
                        /(\r\n|\r|\n)/g,
                        '$1> '
                    );
        },
        IMG: function(elem) {
            return '![alt text](' + elem.src + ')';
        },
        A: function(elem) {
            return '[' + elem.innerHTML + '](' + elem.href + ')';
        },
        UL: function(elem) {
            var li = elem.children;
            var length = li.length;
            var md = '';
            for (var i = 0; i < length; i++) {
                md += ' - ' + indent(li[i].innerHTML) + NEWLINE;
            }
            return md;
        },
        OL: function(elem) {
            var li = elem.children;
            var length = li.length;
            var md = '';
            var start = elem.start|0;
            for (var i = 0; i < length; i++) {
                md += (i + start) + '. ' + indent(li[i].innerHTML) + NEWLINE;
            }
            return md;
        },
        B: function(elem) {
            return '**' + elem.innerHTML + '**';
        },
        STRONG: function(elem) {
            return this.B(elem);
        },
        STRIKE: function(elem) {
            return '---' + elem.innerHTML + '---';
        },
        DEL: function(elem) {
            return this.STRIKE(elem);
        },
        I: function(elem) {
            return '*' + elem.innerHTML + '*';
        },
        PRE: function(elem){
            return indent(this.innerHTML) + NEWLINE;
        },
        CODE: function(elem){
            if(elem.parentNode.tagName != 'PRE')
            {
                return '``' + elem.innerHTML.replace(/\r|\n/g,'').replace(/^\s*(.*)\s*$/,'$1') + '``';
            }
            else
            {
                this.PRE(elem);
            }
        }
    };

    var toMarkdown = function(html){
        var DIV = document.createElement('div'); //will have the HTML to parse.

        DIV.innerHTML = html + '';

        for(var tag in conversions)
        {
            var elements = Array.prototype.slice.call(DIV.getElementsByTagName(tag.toLowerCase()));
            var length = elements.length;
            for(var i = 0; i < length; i++)
            {
                var element = elements[i];

                if(element.childNodes.length > 1)
                {
                    element.innerHTML = toMarkdown(element.innerHTML);
                }

                element.parentNode.replaceChild(
                    document.createTextNode(conversions[tag](element)),
                    element
                );
            }

        }

        return DIV.innerHTML;
    };

    window.MarkdownConverter = {
        addParser: function(tag, fn){
            if( !this.hasParser(tag) )
            {
                conversions[(tag + '').toUpperCase()] = fn;
                return true;
            }
            return false;
        },
        hasParser: function(tag){
            tag = (tag + '').toUpperCase();
            return (tag in conversions);
        },
        fromHTML: function(html){
            return toMarkdown(html + '');
        }
    };

})(Function('return this')());

It exposes a very basic API. The idea is to be as close as I can from the reference.
You can read about it here: https://stackoverflow.com/editing-help

Some mistakes on matching the were fixed. Also, the original version was limited to 1 level only. Which means that things like <p>A <b>bold</b></p> wouldn't produce the right markdown. To fix that, I've added recursion. Also, the <blockquote> only had the markdown on the first line. All those issues and many others (that I don't remember) were fixed.

Another addition was the support for the start attribute (it was deprecated on HTML4.01, but it isn't on HTML5) on ordened lists.

Example of the markdown:

(function(window, undefined){
	'use strict';
	var TEXT_PROPERTY = document.head.innerText ? 'innerText' : 'textContent';
	var NEWLINE = '\r\n';
	
	var indent = function(text){
		return text.replace(/(\r\n|\r|\n)/g,'$1    ');
	};
	
	var conversions = {
        BR: function(){
            return '  ' + NEWLINE;
        },
		P: function(elem) {
			return NEWLINE + NEWLINE + elem.innerHTML + NEWLINE;
		},
		H1: function(elem) {
			return '# ' + elem.innerHTML + NEWLINE;
		},
		H2: function(elem) {
			return '## ' + elem.innerHTML + NEWLINE;
		},
		H3: function(elem) {
			return '### ' + elem.innerHTML + NEWLINE;
		},
		HR: function() {
			//we use 4 to do not cause confusion with <strike></strike>
			return '----' + NEWLINE;
		},
		BLOCKQUOTE: function(elem) {
			return '> ' + elem
                	.innerHTML
                	.replace(
                		/(\r\n|\r|\n)/g,
                		'$1> '
            		);
		},
		IMG: function(elem) {
			return '![alt text](' + elem.src + ')';
		},
		A: function(elem) {
			return '[' + elem.innerHTML + '](' + elem.href + ')';
		},
		UL: function(elem) {
			var li = elem.children;
			var length = li.length;
			var md = '';
			for (var i = 0; i < length; i++) {
				md += ' - ' + indent(li[i].innerHTML) + NEWLINE;
			}
			return md;
		},
		OL: function(elem) {
			var li = elem.children;
			var length = li.length;
			var md = '';
            var start = elem.start|0;
			for (var i = 0; i < length; i++) {
				md += (i + start) + '. ' + indent(li[i].innerHTML) + NEWLINE;
			}
			return md;
		},
		B: function(elem) {
			return '**' + elem.innerHTML + '**';
		},
		STRONG: function(elem) {
			return this.B(elem);
		},
		STRIKE: function(elem) {
			return '---' + elem.innerHTML + '---';
		},
		DEL: function(elem) {
			return this.STRIKE(elem);
		},
		I: function(elem) {
			return '*' + elem.innerHTML + '*';
		},
		PRE: function(elem){
			return indent(this.innerHTML) + NEWLINE;
		},
        CODE: function(elem){
            if(elem.parentNode.tagName != 'PRE')
            {
	            return '``' + elem.innerHTML.replace(/\r|\n/g,'').replace(/^\s*(.*)\s*$/,'$1') + '``';
            }
            else
            {
                this.PRE(elem);
            }
        }
	};
	
	var toMarkdown = function(html){
		var DIV = document.createElement('div'); //will have the HTML to parse.
		
		DIV.innerHTML = html + '';
		
		for(var tag in conversions)
		{
			var elements = Array.prototype.slice.call(DIV.getElementsByTagName(tag.toLowerCase()));
			var length = elements.length;
			for(var i = 0; i < length; i++)
			{
				var element = elements[i];
			
				if(element.childNodes.length > 1)
				{
					element.innerHTML = toMarkdown(element.innerHTML);
				}
				
				element.parentNode.replaceChild(
					document.createTextNode(conversions[tag](element)),
					element
				);
			}
			
		}
		
		return DIV.innerHTML;
	};
	
	window.MarkdownConverter = {
		addParser: function(tag, fn){
			if( !this.hasParser(tag) )
			{
				conversions[(tag + '').toUpperCase()] = fn;
				return true;
			}
			return false;
		},
		hasParser: function(tag){
			tag = (tag + '').toUpperCase();
			return (tag in conversions);
		},
		fromHTML: function(html){
			return toMarkdown(html + '');
		}
	};
	
})(Function('return this')());


document.body.innerHTML=
    '<pre>' + 
    	MarkdownConverter
    		.fromHTML(
				document
					.getElementById('html')
					.innerHTML
			)
    		.replace(/</g,'&lt;')
    		.replace(/>/g,'&gt;') +
    '</pre>';
<div id="html">

  
<p>Te<i><b>s</b></i>t</p>
<h1>h1</h1> 
<br>
<h2>h2</h2> 
<br>
<h3>h3</h3> 
<br>text outside everything
<br>
<h2>(and another element!)</h2> 
<br>
<img src='http://example.com/example.png'>
<br><a href='http://google.com'>a link!</a> 
<br>
<ul>
	<li>item 1</li>
	<li>item 2</li>
	<li>item 3</li>
</ul>
<br>
<ol start="2">
	<li>item 1</li>
	<li>item 2</li>
	<li>item <b>3</b></li>
</ol>
<br><strong>BOLD TEXT</strong> and <i>ITALICISED TEXT</i> 
<br>
<blockquote>blockquote</blockquote>
<br>
  

</div>

\$\endgroup\$
9
  • 1
    \$\begingroup\$ @ᔕᖺᘎᕊ If you want, make a new room and I can try to help on that. \$\endgroup\$ Aug 26, 2015 at 14:58
  • \$\begingroup\$ @ᔕᖺᘎᕊ I've added an alternative implementation. I hope you like it and suits your needs. \$\endgroup\$ Aug 27, 2015 at 22:06
  • 1
    \$\begingroup\$ For the record, from ES5, undefined is read-only. \$\endgroup\$
    – Schism
    Aug 27, 2015 at 22:22
  • \$\begingroup\$ @Schism On the window object, it is. But you can create a local variable and set a value. \$\endgroup\$ Aug 27, 2015 at 22:23
  • 1
    \$\begingroup\$ @ᔕᖺᘎᕊ Don't worry. If you find something that doesn't make you that happy, just drop here a comment, or ping me on The 2nd Monitor. \$\endgroup\$ Sep 1, 2015 at 16:18
8
\$\begingroup\$

A few things to point out:


  ul: function(data) {
    var lis = childnodes[data.i].childNodes;
    var newmd = '';
    var lislength = lis.length;
    for (var x = 0; x < lislength; x++) {
      newmd += "- " + lis[x].innerHTML + "\n";
    }
    return newmd;
  },

You can declare lislength in the for loop, rather than externally, if you like.

for (var x = 0, listlength = list.length; x < listlength; x++) {

Also, note that you should keep your use of quotes consistent ("" & '')

This is especially helpful when trying to avoid re-calling a function, or a method call that is somewhat slow.


  img: function(data) {
    var imgStr = "![alt text](" + data.curEl.src + ")";
    return imgStr;
  },

You can just return imgStr without assigning it.

return "![alt text](" + data.curEl.src + ")";

  strong: function(data) {
    return "**" + data.html + "**";
  },

<strong> is synonymous with <b>, so you may want to consider that, also.


On an abstraction note, you should be passing var doc = new DOMParser().parseFromString(str, 'text/html'); to a class/function, at best, and leave the rest to be handled within the class/function, meaning childnodes and markdown would be better off buried within the structure, rather than as external globals.

\$\endgroup\$
1
  • \$\begingroup\$ Defining variables in the loop has no practical advantages, since all variables are function scope anyway. \$\endgroup\$ Aug 24, 2015 at 13:35
2
\$\begingroup\$

I do believe you need to extend your test regime a bit, as there are quite a few cases you don't cover. Here are some test cases you should consider:

  • Extraneous space within elements – Have you tried stuff like <strong> bolded text </strong>. If this gets translated with the spaces, it will fail...
  • <br>'s within paragraph – Poems, adresses, and some other stuff might use <br> within a paragraph, and as such should be translated into two spaces at the end of the line instead of the paragraph divider or \n\n
  • Nested lists – Lists can, and are often nested. A proper translation should allow for this, and that means you need to keep track of current indentation level
  • Space in front of lists? – Some markdown implementation also require that there is a newline in front of lists for them to be properly formatted. You should have a case or two testing this as well
  • Nested tags – As someone else commented upon in their answer, what about nested tags? I.e. Bold and italics, lists with italics in list elements, blockquote with other tags, ...

What I'm trying to say here, is that you need to extend your test regime quite a bit, and get real life examples when testing. If possible, you should build more complex test scenarios in addition to the happy flow or basic case which you are already testing.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.