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

Improve preprocessor ouput format #3339

Merged

Conversation

chirsz-ever
Copy link
Contributor

There are still some imperfections, for example ++i would be convert to ++ i. I think the current state is good enough.

@CLAassistant
Copy link

CLAassistant commented Sep 22, 2023

CLA assistant check
All committers have signed the CLA.

@chirsz-ever chirsz-ever force-pushed the chirsz/better-pp-output branch from 28c8b81 to 796c94c Compare September 22, 2023 13:22
Copy link
Collaborator

@ncesario-lunarg ncesario-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending CI.

Thanks for the contribution @chirsz-ever. Was there an issue associated with this?

@chirsz-ever
Copy link
Contributor Author

Was there an issue associated with this?

No, I didn't find related issues.

Modify preprocessor.simple.vert to test spaces before parenthesis.
@chirsz-ever chirsz-ever force-pushed the chirsz/better-pp-output branch from 796c94c to e0919b1 Compare September 22, 2023 16:29
@arcady-lunarg arcady-lunarg added the kokoro:run Trigger Google bot runs label Sep 22, 2023
@kokoro-team kokoro-team removed the kokoro:run Trigger Google bot runs label Sep 22, 2023
@arcady-lunarg
Copy link
Contributor

Is this a purely aesthetic change to the appearance of the preprocessor output? Seems like an improvement on that front at least.

@chirsz-ever
Copy link
Contributor Author

chirsz-ever commented Sep 22, 2023

Is this a purely aesthetic change to the appearance of the preprocessor output?

Yes. Most users of glslang do not care about the preprocess output, and glslang's error message only give the line numbers. But I rencently just need the preprocess output for human reading.

In fact IMO the most right scheme is, not reformating code, just keeping the code as what it looks like when inputing it to the preprocessor. Let's fix it in the future when someone really needs it.

@arcady-lunarg arcady-lunarg merged commit 2bfacda into KhronosGroup:main Sep 22, 2023
20 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.

5 participants