search

racz16 / WebGL-GLSL-Editor

WebGL GLSL shader editor extension for Visual Studio Code.
Other
100 stars 12 forks source link

Fix missing diagnostics on macOS by marking validator as executable #36

Closed matt-curtis closed 1 year ago

matt-curtis commented 1 year ago

Thank you for making this extension! It's made my life a whole lot easier (you should add a donate/sponsorship option! :))

This pull request is meant to fix a bug that prevents diagnostics from working on macOS (likely the one referenced by #34.)

In my testing, to fix this it's enough to mark the glslangValidatorMac binary as executable using chmod +x [pathToValidator]. This pull request adds a method, markMacValidatorAsExecutableIfNeeded(), to the GlslDiagnosticProvider class that runs during GlslDiagnosticProvider.initialize().

Edit: Thinking further about this — really the mac validator binary should have the executable bit set to true and that should just be committed to the repository, but since it seems the binaries are manually added to the project I'm not sure what the best way to solve that would be.

racz16 commented 1 year ago

Thank you, I'm glad my extension made your life easier.

I did pretty much the same thing with the Linux compiler that you mentioned in the edit section. I started a virtual machine, cloned the repository, and marked the compiler as an executable, it worked, so I committed. I think this would be the smoothest solution with the Mac version as well, but since I don't have any Apple devices, I can't do it myself. If you can do that and it works, please create a Pull request.

However, if for some reason, it doesn't work with Mac, I think this Pull request is a reasonable solution. I have just 2 comments about it. First, I wouldn't call the markMacValidatorAsExecutableIfNeeded method from the init method of GlslDiagnosticProvider, because it runs every time you open or changes a GLSL file. I would call it when VS Code activates the extension, maybe in the extension.ts's addDiagnostic. The other thing is just nitpicking, but in the code, I always use curly braces after if statements, so it would be nice to have it in markMacValidatorAsExecutableIfNeeded.

But ultimately I would prefer the first solution, without the code changes.

matt-curtis commented 1 year ago

Closing this pull request! I'm going to open a separate one with the binary flag.