rojo-rbx / rojo

Rojo enables Roblox developers to use professional-grade software engineering tools
https://rojo.space
Mozilla Public License 2.0
906 stars 170 forks source link

Correct issue with default.project.json files with no name being named `default` after change #917

Open Dekkonot opened 1 month ago

Dekkonot commented 1 month ago

Closes #916.

Turns out we had a test that should've caught this, it's just that nobody caught it when this feature was implemented. We'll likely want to backport this into the 7.4.x branch as well.

Let me know if you have a better way to calculate project names in the middleware, I'm not in love with the way it's implemented here.

kennethloeffler commented 1 month ago

So there is a problem with the above approach: sync rules mean that project files won't necessarily have .project.json as a suffix, meaning that such files must contain a name field, otherwise the project will fail to load with ProjectNameInvalid. I think we should still allow a missing name field in this case (it should also have a test!). I'd still like the optional name logic to be defined in one place, so maybe we should give the project load functions a name parameter to fall back to in case we can't strip the .project.json suffix?

Dekkonot commented 1 month ago

I went through and redid the name resolution and it's now generalized. As long as we use the right methods on Project, it'll always work the same now.

In the process, I did modify project loading to always use a Vfs, which impacts the fmt-project subcommand. Nothing invasive though.