Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing bug about outputting "undefined" bullets #192

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ventus218
Copy link

To be honest I'm not a JavaScript guy but I think that I may have fixed the problem seen in #42, #131 and #168.

The actual output of "undefined" was made here

res.push(listitem(lvl, ele.content, opts));

Basically some elements had their lvl negative and so the bullet was "undefined".

I think that the source of the problem lies in two different places.

function bullets(arr, options) {
  var opts = utils.merge({indent: '  '}, options);
  opts.chars = opts.chars || opts.bullets || ['-', '*', '+'];
  var unindent = 0;

  var listitem = utils.li(opts);
  var fn = typeof opts.filter === 'function'
    ? opts.filter
    : null;

  // (1) HERE
  // Keep the first h1? This is `true` by default
  if (opts && opts.firsth1 === false) {
    unindent = 1;
  }

  var len = arr.length;
  var res = [];
  var i = 0;

  while (i < len) {
    var ele = arr[i++];
    // (1) HERE
    ele.lvl -= unindent;
    if (fn && !fn(ele.content, ele, arr)) {
      continue;
    }

    if (ele.lvl > opts.maxdepth) {
      continue;
    }

    var lvl = ele.lvl - opts.highest;
    res.push(listitem(lvl, ele.content, opts));
  }
  return res.join('\n');
}

Unindenting (1) just because the first H1 needs to be stripped doesn't work well in a document which has others H1s down the line.

The second problem lies here:

function generate(options) {
  var opts = utils.merge({firsth1: true, maxdepth: 6}, options);
  var stripFirst = opts.firsth1 === false;
  if (typeof opts.linkify === 'undefined') opts.linkify = true;

  return function(md) {
    md.renderer.render = function(tokens) {
      tokens = tokens.slice();
      var seen = {};
      var len = tokens.length, i = 0, num = 0;
      var tocstart = -1;
      var arr = [];
      var res = {};

      while (len--) {
        var token = tokens[i++];
        if (/<!--[ \t]*toc[ \t]*-->/.test(token.content)) {
          tocstart = token.lines[1];
        }

        if (token.type === 'heading_open') {
          tokens[i].lvl = tokens[i - 1].hLevel;
          tokens[i].i = num++;
          arr.push(tokens[i]);
        }
      }

      var result = [];
      res.json = [];

      // exclude headings that come before the actual
      // table of contents.
      var alen = arr.length, j = 0;
      while (alen--) {
        var tok = arr[j++];

        if (tok.lines && (tok.lines[0] > tocstart)) {
          var val = tok.content;
          if (tok.children && tok.children[0].type === 'link_open') {
            if (tok.children[1].type === 'text') {
              val = tok.children[1].content;
            }
          }

          if (!seen.hasOwnProperty(val)) {
            seen[val] = 0;
          } else {
            seen[val]++;
          }

          tok.seen = opts.num = seen[val];
          tok.slug = utils.slugify(val, opts);
          res.json.push(utils.pick(tok, ['content', 'slug', 'lvl', 'i', 'seen']));
          if (opts.linkify) tok = linkify(tok, opts);
          result.push(tok);
        }
      }
      // (2) HERE
      opts.highest = highest(result);
      res.highest = opts.highest;
      res.tokens = tokens;

      // (3) HERE
      if (stripFirst) result = result.slice(1);
      res.content = bullets(result, opts);
      res.content += (opts.append || '');
      return res;
    };
  };
}

Computing the highest (2) heading and THEN stripping out the first h1 means that we are computing about old data. Furthermore (3) we are assuming that the first H1 will be the first in the array but that is not always the case, think about this input:

# HelloH1
<!-- toc -->
## HelloH2
# HelloH1_2

So I propose to move up the stripping of the first H1 and actually searching for an H1 only. And then get rid of the unindent step.

Feel free to correct my JS or to tell me if there are some edge cases that I didn't think about (btw all tests are successful)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants