filter function still called with directories when nodir: true

[nicolasroger17]

I just updated to version 4 (was on version 2).

I noticed that with the option nodir: true, the library was actually looking only at the files, which is expected, but unfortunately it does so only at depth 0, and never goes inside of the directories. It is a bit of a problem, because if one wants to find specific files (using a filter function), you will need instead to do something like:

const path = require('path');
const klawSync = require('klaw-sync');

function filterFn(item) {
  return item.stats.isDirectory() || /[some regex]/.test(path.basename(item.path));
}

const paths = klawSync([FOLDER_PATH], { filter: filterFn });

and then you will need to loop on the paths one more time to filter the directories out yourself.

const files = paths.filter(function(item) {
  return item.stats.isFile();
});

It would great if the default with nodir: true was to traverse all the directories, and call the filter function only for the files.

[manidlou]

@nicolasroger17 it should traverse directories while nodir: true. But, seems like depth limit implementation is buggy and needs to be fixed.

[manidlou]

Fixed in v5.0.0.

[jskrzypek]

@manidlou @Geelik should we re-open this? The depthLimit looks like it's working, but the original problem of filtering directories even with nodir: true noted by @nicolasroger17 is still not resolved.

His example still doesn't work (with or without) nodir: true if there's no item.stats.isDirectory() || in the filter function.

But maybe a better solution than simply always traversing when nodir: true would be to another option called traverseAll that would ensure all directories are traversed regardless of nodir. This would give the benefit of providing backwards compatibility with the current behavior.

This could be achieved by building on #12 and further simplifying the logic in the for loop like so:

  for (var i = 0; i < paths.length; i += 1) {
    const pi = paths[i];
    const st = opts.fs.statSync(pi);
    const item = { path: pi, stats: st };
    const isUnderDepthLimit = (!opts.rootDepth || pi.split(path.sep).length - opts.rootDepth < opts.depthLimit);
    const filterResult = opts.filter ? opts.filter(item) : true;
    const isDirectory = st.isDirectory();
    const shouldAdd = filterResult && (isDirectory ? !opts.nodir : !opts.nofile);
    const shouldTraverse = isDirectory && isUnderDepthLimit && (opts.traverseAll || filterResult);

    if (shouldAdd) {
        ls.push(item);
    }
    if (shouldTraverse) {
        ls = klawSync(pi, opts, ls);
    }
  }

If you think that makes sense I'll be happy to make a PR.

[Geelik]

Yeah i think adding an option is a good idea, it will still allow to filtered out directories with filter if needed and to not bother if we are sure we want to traverse all the tree.

[manidlou]

PR welcome!

[jskrzypek]

@manidlou PR made! 😄

[manidlou]

Released in v6.0.0.