Skip to content

Commit

Permalink
Change footnote order
Browse files Browse the repository at this point in the history
Previously, the footer (if footnote definitions were found) showed all footnote
definitions, in definition order.  Even footnote definitions with the same
identifier were transformed to multiple definitions.

remarkjs/remark-rehype#11 shows that definition order does not make sense if
inline footnotes (`[^Some text]`) are used in combination with footnote
references (`[^id]`) and footnote definitions (`[^id]: Some text`).

Investigating this, I came to the conclusion that it also does not make sense to
output all definitions, whether they are used (or even duplicate) or not.

Instead, it makes sense to track the order in which footnotes are used (first),
and only output a definition for the used inline footnotes / footnote
references, in the order they were used.  This is in line with how link
definitions are used as well.

Closes remarkjs/remark-rehype#11.
Closes GH-32.
  • Loading branch information
wooorm authored May 14, 2019
1 parent 751b54d commit fd38c45
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 31 deletions.
24 changes: 15 additions & 9 deletions lib/footer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,23 @@ var list = require('./handlers/list')
var wrap = require('./wrap')

function generateFootnotes(h) {
var footnotes = h.footnotes
var length = footnotes.length
var footnoteById = h.footnoteById
var footnoteOrder = h.footnoteOrder
var length = footnoteOrder.length
var index = -1
var listItems = []
var def
var backReference
var content
var tail

if (!length) {
return null
}

while (++index < length) {
def = footnotes[index]
def = footnoteById[footnoteOrder[index].toUpperCase()]

if (!def) {
continue
}

content = def.children.concat()
tail = content[content.length - 1]
backReference = {
Expand All @@ -38,12 +40,16 @@ function generateFootnotes(h) {

tail.children.push(backReference)

listItems[index] = {
listItems.push({
type: 'listItem',
data: {hProperties: {id: 'fn-' + def.identifier}},
children: content,
position: def.position
}
})
}

if (listItems.length === 0) {
return null
}

return h(
Expand Down
5 changes: 5 additions & 0 deletions lib/handlers/footnote-reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@ module.exports = footnoteReference
var u = require('unist-builder')

function footnoteReference(h, node) {
var footnoteOrder = h.footnoteOrder
var identifier = node.identifier

if (footnoteOrder.indexOf(identifier) === -1) {
footnoteOrder.push(identifier)
}

return h(node.position, 'sup', {id: 'fnref-' + identifier}, [
h(node, 'a', {href: '#fn-' + identifier, className: ['footnote-ref']}, [
u('text', node.label || identifier)
Expand Down
19 changes: 8 additions & 11 deletions lib/handlers/footnote.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,25 @@ module.exports = footnote
var footnoteReference = require('./footnote-reference')

function footnote(h, node) {
var identifiers = []
var footnoteById = h.footnoteById
var footnoteOrder = h.footnoteOrder
var identifier = 1
var footnotes = h.footnotes
var length = footnotes.length
var index = -1

while (++index < length) {
identifiers[index] = footnotes[index].identifier
}

while (identifiers.indexOf(String(identifier)) !== -1) {
while (identifier in footnoteById) {
identifier++
}

identifier = String(identifier)

footnotes.push({
// No need to check if `identifier` exists, as it’s guaranteed to not exist.
footnoteOrder.push(identifier)

footnoteById[identifier] = {
type: 'footnoteDefinition',
identifier: identifier,
children: [{type: 'paragraph', children: node.children}],
position: node.position
})
}

return footnoteReference(h, {
type: 'footnoteReference',
Expand Down
24 changes: 17 additions & 7 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,22 @@ var one = require('./one')
var footer = require('./footer')
var handlers = require('./handlers')

var own = {}.hasOwnProperty

// Factory to transform.
function factory(tree, options) {
var settings = options || {}
var dangerous = settings.allowDangerousHTML
var footnoteById = {}

h.dangerous = dangerous
h.definition = definitions(tree, settings)
h.footnotes = []
h.footnoteById = footnoteById
h.footnoteOrder = []
h.augment = augment
h.handlers = xtend(handlers, settings.handlers || {})

visit(tree, 'footnoteDefinition', visitor)
visit(tree, 'footnoteDefinition', onfootnotedefinition)

return h

Expand Down Expand Up @@ -80,19 +84,25 @@ function factory(tree, options) {
})
}

function visitor(definition) {
h.footnotes.push(definition)
function onfootnotedefinition(definition) {
var id = definition.identifier.toUpperCase()

// Mimick CM behavior of link definitions.
// https://github.com/syntax-tree/mdast-util-definitions/blob/8d48e57/index.js#L26
if (!own.call(footnoteById, id)) {
footnoteById[id] = definition
}
}
}

// Transform `tree`, which is an mdast node, to a hast node.
function toHast(tree, options) {
var h = factory(tree, options)
var node = one(h, tree)
var footnotes = footer(h)
var foot = footer(h)

if (node && node.children && footnotes) {
node.children = node.children.concat(u('text', '\n'), footnotes)
if (foot) {
node.children = node.children.concat(u('text', '\n'), foot)
}

return node
Expand Down
62 changes: 58 additions & 4 deletions test/footnote-definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,69 @@ var u = require('unist-builder')
var to = require('..')

test('FootnoteDefinition', function(t) {
t.equal(
t.deepEqual(
to(
u('footnoteDefinition', {identifier: 'zulu'}, [
u('paragraph', [u('text', 'alpha')])
u('root', [
u('footnoteDefinition', {identifier: 'zulu'}, [
u('paragraph', [u('text', 'alpha')])
])
])
),
null,
u('root', []),
'should ignore `footnoteDefinition`'
)

t.deepEqual(
to(
u('root', [
u('footnoteReference', {identifier: 'zulu'}),
u('footnoteDefinition', {identifier: 'zulu'}, [
u('paragraph', [u('text', 'alpha')])
]),
u('footnoteDefinition', {identifier: 'zulu'}, [
u('paragraph', [u('text', 'bravo')])
])
])
),
u('root', [
u('element', {tagName: 'sup', properties: {id: 'fnref-zulu'}}, [
u(
'element',
{
tagName: 'a',
properties: {href: '#fn-zulu', className: ['footnote-ref']}
},
[u('text', 'zulu')]
)
]),
u('text', '\n'),
u('element', {tagName: 'div', properties: {className: ['footnotes']}}, [
u('text', '\n'),
u('element', {tagName: 'hr', properties: {}}, []),
u('text', '\n'),
u('element', {tagName: 'ol', properties: {}}, [
u('text', '\n'),
u('element', {tagName: 'li', properties: {id: 'fn-zulu'}}, [
u('text', 'alpha'),
u(
'element',
{
tagName: 'a',
properties: {
href: '#fnref-zulu',
className: ['footnote-backref']
}
},
[u('text', '↩')]
)
]),
u('text', '\n')
]),
u('text', '\n')
])
]),
'should use the first `footnoteDefinition` if multiple exist'
)

t.end()
})
91 changes: 91 additions & 0 deletions test/footnote-mixed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
'use strict'

var test = require('tape')
var u = require('unist-builder')
var to = require('..')

test('Footnote', function(t) {
var mdast = to(
u('root', [
u('paragraph', [
u('text', 'Alpha'),
u('footnote', [u('text', 'Charlie')])
]),
u('paragraph', [
u('text', 'Bravo'),
u('footnoteReference', {identifier: 'x'})
]),
u('footnoteDefinition', {identifier: 'x'}, [
u('paragraph', [u('text', 'Delta')])
])
])
)

var hast = u('root', [
u('element', {tagName: 'p', properties: {}}, [
u('text', 'Alpha'),
u('element', {tagName: 'sup', properties: {id: 'fnref-1'}}, [
u(
'element',
{
tagName: 'a',
properties: {href: '#fn-1', className: ['footnote-ref']}
},
[u('text', '1')]
)
])
]),
u('text', '\n'),
u('element', {tagName: 'p', properties: {}}, [
u('text', 'Bravo'),
u('element', {tagName: 'sup', properties: {id: 'fnref-x'}}, [
u(
'element',
{
tagName: 'a',
properties: {href: '#fn-x', className: ['footnote-ref']}
},
[u('text', 'x')]
)
])
]),
u('text', '\n'),
u('element', {tagName: 'div', properties: {className: ['footnotes']}}, [
u('text', '\n'),
u('element', {tagName: 'hr', properties: {}}, []),
u('text', '\n'),
u('element', {tagName: 'ol', properties: {}}, [
u('text', '\n'),
u('element', {tagName: 'li', properties: {id: 'fn-1'}}, [
u('text', 'Charlie'),
u(
'element',
{
tagName: 'a',
properties: {href: '#fnref-1', className: ['footnote-backref']}
},
[u('text', '↩')]
)
]),
u('text', '\n'),
u('element', {tagName: 'li', properties: {id: 'fn-x'}}, [
u('text', 'Delta'),
u(
'element',
{
tagName: 'a',
properties: {href: '#fnref-x', className: ['footnote-backref']}
},
[u('text', '↩')]
)
]),
u('text', '\n')
]),
u('text', '\n')
])
])

t.deepEqual(mdast, hast, 'should order the footnote section by usage')

t.end()
})
1 change: 1 addition & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require('./delete.js')
require('./emphasis.js')
require('./footnote-definition.js')
require('./footnote-reference.js')
require('./footnote-mixed.js')
require('./footnote.js')
require('./heading.js')
require('./html.js')
Expand Down

0 comments on commit fd38c45

Please sign in to comment.