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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix file.exclude for complex extensions #56893

Closed
wants to merge 2 commits into from

Conversation

kutyel
Copy link

@kutyel kutyel commented Aug 21, 2018

Fixes #56812

@isidorn I think it is pretty straight forward 馃槈 Although the new test is having a timeout don't know exactly why 馃槙 should be easy to fix though!

@msftclas
Copy link

msftclas commented Aug 21, 2018

CLA assistant check
All CLA requirements met.

@isidorn
Copy link
Contributor

isidorn commented Aug 21, 2018

No, this fix does not really fix it. The extname has an expected behavior which you just changed
For more details check out the node docs https://nodejs.org/api/path.html#path_path_extname_path

@kutyel
Copy link
Author

kutyel commented Aug 21, 2018

@isidorn you are right, hope you like it better now 馃檪 would you please help me to figure out why the test does not work? I managed to debug locally and it works but the test does not finish 馃槙

@isidorn
Copy link
Contributor

isidorn commented Aug 22, 2018

This is changing the functionality of glob which can affect many other things. So again this fix is not good imho.
Though forwarding to @bpasero since he owns the glob.

Unfortunetly I do not have time right now to look into why the test is failing.

@isidorn isidorn assigned bpasero and unassigned isidorn Aug 22, 2018
@bpasero
Copy link
Member

bpasero commented Aug 22, 2018

Closing for the reasons mentioned. I think we would need to introduce a new syntax for this to not break backwards compatibility.

@bpasero bpasero closed this Aug 22, 2018
@kutyel kutyel deleted the bugfix/basename-with-dot branch August 22, 2018 09:09
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explorer should support basename with dot
4 participants