Stale source links in readme reference wrong line

[mootari]

Source links were added in 1ba39e7 but reference the master branch. Because the code has changed since that commit many (if not all) source links now reference arbitrary lines.

Proposed solutions:

1. Remove source links. It's not that hard to search the source, although the links provide a good incentive to actually look there.
2. Link to the code search (example for scaleExtent). Quality of results might vary.
3. Devise a process to automatically update source links.

[mootari]

Option 3 is probably not feasible for many reasons. I'd recommend option 2.

It's probably also fair to assume that this issue applies to other d3 modules as well.

[mbostock]

Thanks for the feedback. This was a consideration when the source links were added, but I’d rather have broken links than no links at all. If you want to devise a process to update the links automatically, that’d be great!

[mbostock]

The code search link is an interesting idea, although the problem is it then requires two clicks to read the source. And it’ll still break if code moves between files or files are renamed. If the source links are off by a few lines, usually it won’t be that hard to find the definition.

Anyhow, of course it’d be great to automate this. And not just the source links, but the method definitions and the table of contents. But all of this is maintained by hand at the moment.

[mootari]

During some (late) spring data cleaning I rediscovered code I had already written that tracks line positions across commits and dumps changes to stdout. Dropping the code here for reference.

Example output (excerpt):

Source                                               | Old | New
==================================================== | === | ===
  zoom.transform = function(collection, transform) { | 62  | 84 
  zoom.transform = function(collection, transform) { | 76  | 84 
  zoom.translateBy = function(selection, x, y) {     | 110 | 118
  zoom.translateTo = function(selection, x, y) {     | 119 | 127
  zoom.scaleBy = function(selection, k) {            | 91  | 99 
  zoom.scaleTo = function(selection, k) {            | 99  | 107

Node script:

var fs = require('fs');
var execSync = require('child_process').execSync;
var readline = require('readline');

options = {
  dir: 'd3-zoom',
  file: 'README.md',
  prefix: 'https://github.com/d3/d3-zoom/blob/master/',
  delimiter: '#L',
  suffix: ' ',
  updateFile: false
};

function findSourceLinks(data, prefix, delimiter, suffix) {
  function esc(str) {
    return str.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&");
  }
  var pattern, lines, matches = [];

  pattern = new RegExp(esc(prefix) + '(.+?)' + esc(delimiter) + '([0-9]+)' + esc(suffix), 'g');
  lines = data.split('\n');

  lines.forEach(function(str, index) {
    var match;
    pattern.lastIndex = 0;
    while(match = pattern.exec(str)) {
      matches.push({
        lineContent: str,
        lineIndex: index,
        linkIndex: match.index,
        targetUrl: match[0],
        targetFile: match[1],
        targetLine: +match[2]
      });
    }
  });

  return matches;
}

/**
 * Finds the first commit that introduced a string into a file.
 *
 * @param {string} gitCmd - Base git command.
 * @param {string} str - Search string.
 * @param {string} file - File path.
 * @returns {string}|{null} First commit ID or null if none was found.
 */
function gitStringOrigin(gitCmd, str, file) {
  var result, match;

  result = execSync(gitCmd + ' log --reverse --oneline -S "' + str + '" -- "' + file + '"');
  match = result.toString().match(/^[a-f0-9]{5,}/);
  return match ? match[0] : null;
}

/**
 * Returns the content of a file's line in a specific commit.
 *
 * @param {string} gitCmd - Base git command.
 * @param {string} commit - Commit ID.
 * @param {string} file - File path.
 * @param {string} line - Line number.
 * @returns {string}|{null} - Either the line content or null if not found.
 */
function stringGitSource(gitCmd, commit, file, line) {
  var result, lines;

  result = execSync(gitCmd + ' show ' + commit + ':"' + file + '"');
  lines = result.toString().split('\n');
  return lines.length >= line ? lines[line - 1] : null;
}

/**
 * Renders a formatted table.
 * @param {string[]} headers - Header labels.
 * @param {string[][]} rows - Row data.
 * @returns {string} - Rendered table.
 */
function renderTable(headers, rows) {
  /**
   * Returns column widths for a set of rows.
   * @param {string[][]} rows - Rows with column data.
   * @returns {number[]} - Maximum width of each column.
   */
  function colWidths(rows) {
    return rows.reduce(function(sizes, row) {
      return row.map(function(str, i) {
        str += '';
        return sizes[i] && sizes[i] > str.length ? sizes[i] : str.length;
      });
    }, []);
  }

  /**
   * Extends a string to a given length.
   * @param {string} str
   * @param {number} length - Target length.
   * @param {string} char - Character to append.
   * @returns {string}
   */
  function fill(str, length, char) {
    str += '';
    while(str.length < length) {
      str += char;
    }
    return str;
  }

  /**
   * Renders a single row.
   * @param {number[]} widths - Column sizes.
   * @param {string[]} items - Column data.
   * @param {string} fillChar - Character to fill up short columns with.
   * @param {string} delimiter - Column delimiter.
   * @returns {string} - Rendered column.
   */
  function renderRow(widths, items, fillChar, delimiter) {
    return items.map(function(str, i) {
      return fill(str, widths[i], fillChar);
    }).join(delimiter);
  };

  var widths = colWidths([headers].concat(rows));
  var output = [];
  output.push(renderRow(widths, headers, ' ', ' | '));
  output.push(renderRow(widths, headers.fill(''), '=', ' | '));
  rows.forEach(function(row) {
    output.push(renderRow(widths, row, ' ', ' | '));
  });

  return output.join('\n');
}

targetFile = options.dir + '/' + options.file;

var reader = readline.createInterface({
  input: fs.createReadStream(targetFile)
});
reader.on('line', function(data) {

});

fs.readFile(targetFile, 'UTF-8', function(err, data) {
  var matches, lines, cmd, changes = [];

  matches = findSourceLinks(data, options.prefix, options.delimiter, options.suffix);
  cmd = 'git -C "' + options.dir + '"';
  matches.forEach(function(match) {
    var commit = gitStringOrigin(cmd, match.targetUrl, options.file);
    if(commit === null) {
      return;
    }
    match.linkCommit = commit;
    match.targetSource = stringGitSource(cmd, match.linkCommit, match.targetFile, match.targetLine);

    filedata = fs.readFileSync(options.dir + '/' + match.targetFile).toString().split('\n');
    filedata.some(function(line, index) {
      if(line.indexOf(match.targetSource) >= 0) {
        match.targetLineNew = index + 1;
        return true;
      }
    });
  });

  lines = data.split('\n');
  matches.forEach(function(match) {
    var parts, updated;
    if(match.targetLineNew !== match.targetLine) {
      parts = match.targetUrl.split(options.delimiter, 2);
      updated = parts[0] + options.delimiter + match.targetLineNew + options.suffix;
      lines[match.lineIndex] = lines[match.lineIndex].replace(match.targetUrl, updated);
      changes.push([
        match.targetSource,
        match.targetLine,
        match.targetLineNew
      ]);
    }
  });

  if(changes.length) {
    if(options.updateFile) {
      console.log('Updating ' + targetFile);
      fs.writeFile(targetFile, lines.join('\n'), function(err) {
        if(err) {
          console.log(err);
        }
      });
    }
    console.log(renderTable(['Source', 'Old', 'New'], changes));
  }
  else {
    console.log('No changes found.');
  }

});