forge42dev / Remix-Forge

VS Code extension that allows you to generate configurable Remix code
MIT License
78 stars 6 forks source link

fix to execute `npx shadcn@latest` command #17

Closed kbwo closed 7 months ago

kbwo commented 8 months ago

At first, thank you for great extension.

I fixed #3 and #16.

I have done as much testing as possible, but I have concerns regarding the following points related to the changes:

I apologize, the commits became a bit sloppy because I did this for myself. If you'd prefer separate commits and PRs for each issue, I can resubmit them that way.

AlemTuzlak commented 8 months ago

Thank you so much, can you tell me what you tested on? Windows or mac? Will be testing it myself I already have stuff ready to go to test this out

kbwo commented 8 months ago

@AlemTuzlak Thank you for your quick reply. I tested on Linux. I can also test on a Mac, but I'm currently short on time. If testing on a Mac is necessary, you'll have to wait for a while.

AlemTuzlak commented 8 months ago

@kbwo I'll test on windows would really appreciate mac too! Will try to find someone if you don't have the time

kbwo commented 8 months ago

@AlemTuzlak Sorry for the delay. I have conducted tests on a Mac, and everything was working without issues in both of normal repository and nested directory like monorepo. There were also some parts that seem to be unrelated to the current fixes, but since I am not fully familiar with all the features of this extension, I tested all the things listed under Features in the README. I don't have a Windows machine, so unfortunately, I won't be able to test it on that platform.

AlemTuzlak commented 8 months ago

@kbwo I have the windows machine so I can test it there, and I did, so the #16 is fixed but #3 is not, the issue with #3 was when you run commands in a subdirectory of a monorepo the code is generated on the root of the monorepo instead of the subdirectory, If you look at: https://github.com/Code-Forge-Net/Remix-Forge/compare/main...3-should-be-monorepo-aware I almost got it working but I haven't thoroughly tested every action after this, but the idea should be that the find goes from the current folder upwards instead of the root downwards. Would you mind changing that?

kbwo commented 8 months ago

@AlemTuzlak Ah, I misunderstood the content of the issue #3 a bit. The problem you pointed out was reproducible in my environment as well. It becomes particularly buggy when there are multiple Remix projects in one repository. I will fix the code to resolve the issue. Thank you for your feedback.

kbwo commented 8 months ago

@AlemTuzlak I've changed codes for #3, and tested with repository which has multiple, nested Remix directories.

The idea should be that the find goes from the current folder upwards instead of the root downwards.

I have made changes to the code according to the policy you suggested. One thing to note is that in the case of a monorepo, if the workspace is not the Remix directory, checkRemixVersion will not be triggered upon activate. (This is considering the existence of multiple Remix projects)

I tested on Linux and mac again. I apologize for the inconvenience, but I would like you to confirm that it works on Windows again.

AlemTuzlak commented 7 months ago

@kbwo I am so sorry I've been super busy and had some personal problems lately, I'll be going over this very soon! Thank you again for your work its really appreciated