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 pp.join([...]), which joins an array of strings, preserving vertical alignment #360

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kainino0x
Copy link
Collaborator

And use pp in a few places.

Fixes #356

@kainino0x kainino0x requested a review from austinEng November 18, 2020 02:18
Copy link
Collaborator Author

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

WDYT?

${useBindGroup1 ? '[[set 1, binding 0]] var<image> image1;' : ''}
const wgslFragment = pp`
${pp._if(useBindGroup0)}[[set 0, binding 0]] var<image> image0;${pp._endif}
${pp._if(useBindGroup1)}[[set 1, binding 0]] var<image> image1;${pp._endif}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe not better.

@austinEng
Copy link
Collaborator

Hmm yea I don't see so much value in having it join lines by preserving indentation.

Overall, I think what's most important is readability of the template code which is better both without pp.join and without manual string formatting.

What could be neat though is a simple post process formatter to fixup indentation for logging purposes?

@kainino0x
Copy link
Collaborator Author

Maintaining the indentation is mostly just incidental since I could do it easily. Originally, I was going to have any interpolant of type string[] automatically get joined by \n, but it seemed better to be explicit (at which point it's not really better than .join('\n'), I guess). Do you think it would be more useful that way?

manual string formatting

what do you mean by this?

@austinEng
Copy link
Collaborator

Maintaining the indentation is mostly just incidental since I could do it easily. Originally, I was going to have any interpolant of type string[] automatically get joined by \n, but it seemed better to be explicit (at which point it's not really better than .join('\n'), I guess). Do you think it would be more useful that way?

I do think it would be easier to use. Seems like pp.join is roughly as much typing as .join('\n')

manual string formatting

what do you mean by this?

Doing .join('\n')

@kainino0x
Copy link
Collaborator Author

I may not keep this but if I do something like this, TODO: update join in setVertexBuffer.spec.ts

@kainino0x kainino0x marked this pull request as draft February 2, 2021 03:48
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.

preprocessor: Join arrays of strings with '\n' for convenience
2 participants