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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add file to context if it really exits but could not be expanded by glob, which is the case of pages/[slug]/index.tsx or simillar pattern, common in nextjs #301

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

rogeriochaves
Copy link
Contributor

I was getting errors when trying to include files like [that].tsx (yes, this is allowed)

This commit fixes it

…lob, which is the case of pages/[slug]/index.tsx or simillar pattern, common in nextjs
@jakethekoenig
Copy link
Member

jakethekoenig commented Nov 18, 2023

Thanks for the fix! I think it has a bug for paths with colons though e.g. test:[example].tsx won't work. I think it'd be better to have the check right after the if new_paths.

That'll also have a bug if the glob has matches and exists. I'm not sure how to handle that case actually because it seems like the user might want any of the three possibilities. Probably adding the glob files and not the actual file is sensible though. actually I think adding the actual file and not the glob matches makes the most sense so maybe the check should be first in the for loop.

@PCSwingle
Copy link
Member

Let's make sure we also make sure to add a test for this (maybe even one with a colon as well)? I think we might be changing this logic relatively soon and it would be good to make sure this can't slip through again. I can handle finishing up this PR after I finish what I'm currently working on if nobody else has done it.

@PCSwingle PCSwingle merged commit b4a88c4 into AbanteAI:main Nov 21, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants