-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
fix: index file judgement bug (#306) #308
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -236,7 +236,7 @@ async function genComponentRegistrationFile ({ sourceDir }) { | |||
return `import Vue from 'vue'\n` + components.map(genImport).join('\n') | |||
} | |||
|
|||
const indexRE = /\b(index|readme)\.md$/i | |||
const indexRE = /(?<=(^|\/))(index|readme)\.md$/i | |||
const extRE = /\.(vue|md)$/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lookbehind
is good, how about /(^|\/)(index|readme)\.md$/i
? it's same for this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not the same. \b
will not include the word boundary itself. But /(^|\/)(index|readme)\.md$/i
will include the begining /
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant /(^|\/)(index|readme)\.md$/i
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant /(^|\/)(index|readme)\.md$/i
too... /(^|\/)(index|readme)\.md$/i
has different behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, in other words, \b
and (?<=(^|\/))
won't include the begining /
, but (^|\/)
will.
For instance:
If the path is 'config/README.md':
/\b(index|readme)\.md$/i
=> 'README.md'/(?<=(^|\/))(index|readme)\.md$/i
=> 'README.md'/(^|\/)(index|readme)\.md$/i
=> '/README.md'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, thanks for the explanation. I thought that the indexRE
is only used to test
, after double confirm, I find it's also used to replace
so this is not the same. cool!
Alternative code: const indexRE = /(^|\/)(index|readme)\.md$/i
// ...
// README.md -> /
// foo/README.md -> /foo/
const fileReplace = file.replace(indexRE, '')
return '/' + ( fileReplace === '' ? '' : fileReplace + '/') |
#306