return absolute paths only if project root is specified #11
Conversation
|
Вегеда еще сразу сюда призывай :) |
There was a problem hiding this comment.
Слушай, а может мы еще и вот эту фигню выпилим заодно, если уж и так и так ломаем обратную совместимость? https://github.com/gemini-testing/glob-extra/blob/master/lib/index.js#L15-L17
Просто этот костыль был добавлен специально для того, чтобы не обрабатывать это в gemini, но с новой архитектурой сетов там и так все хорошо обрабатывается, то есть в эту часть кода в glob-extra мы по факту никогда не заходим.
| @@ -3,7 +3,6 @@ const _ = require('lodash'); | |||
|
|
|||
| module.exports = (options) => { | |||
| return _.defaults(options || {}, { | |||
| formats: [], | |||
| root: process.cwd() | |||
There was a problem hiding this comment.
можно в defaults явно указать, что root: null , то есть не задается
| path = qfs.join(options.root, path); | ||
| } | ||
|
|
||
| return utils.isFile(path) | ||
| .then((isFile) => isFile ? [path] : listFiles(path)) | ||
| .then((paths) => paths.filter((path) => utils.matchesFormats(path, options.formats))) | ||
| .then((paths) => paths.map((path) => qfs.absolute(path))); | ||
| .then((paths) => options.root ? paths.map((path) => qfs.absolute(path)) : paths); |
There was a problem hiding this comment.
а нам точно нужна эта строчка?
мы же вроде как уже зарезолвили путь...
There was a problem hiding this comment.
Тогда вот такие кейсы не будут работать
https://github.com/gemini-testing/glob-extra/pull/11/files#diff-0fd0e07cf6d02bf7cf00f18cebb8e6eaR79
There was a problem hiding this comment.
вот это давай голосом обсудим.. что-то я не понял на ночь глядя, в чем там проблема 😄
| @@ -24,14 +24,14 @@ const listFiles = (path) => { | |||
| }; | |||
|
|
|||
| const expandPath = (path, options) => { | |||
| if (!qfs.isAbsolute(path)) { | |||
| if (!qfs.isAbsolute(path) && options.root) { | |||
There was a problem hiding this comment.
а вроде вот так должно работать:
path = options.root ? path.resolve(options.root, path) : path;попробуй, плз.
тут так тестить:
- не передавать
root, но передать относительные пути
2 не передаватьroot, но передавать абсолютные пути - передавать
rootи передать относительные пути - передавать
rootи передавать сразу абсолютные пути (изначально переданные абсолютные пути никак не должны объединяться сroot-ом)
| @@ -56,4 +56,3 @@ exports.isMask = isMask; | |||
|
|
|||
| // :TODO: Backward compatibility | |||
| exports.isMasks = (patterns) => [].concat(patterns).every(isMask); | |||
There was a problem hiding this comment.
ты все равно будешь выпускать мажор!!! можешь отрывать
| .then((absolutePaths) => { | ||
| assert.deepEqual(absolutePaths, ['/absolute/some/deep/path/file.js']); | ||
| .then((paths) => { | ||
| assert.deepEqual(paths, ['some/deep/path/file.js']); |
|
|
||
| return globExtra.expandPaths(['some/path/*.*'], {root: 'root'}) | ||
| .then((paths) => { | ||
| assert.deepEqual(paths, ['/absolute/root/some/path/file.js']); |
Я бы оставил, чтобы модуль был более универсальным. Мало ли, как мы его в будущем захотим использовать. |
|
Вот этот коммент пропустил или не согласен?
? |
|
Я ответил выше на этот коммент #11 (comment) |
договорились голосом, что это делать надо |
rostik404
force-pushed
the
fix-absolute-expanding
branch
2 times, most recently
from
5b6b117
to
3732005
Compare
/cc @j0tunn, @sipayRT, @eGavr, @tormozz48, @DudaGod