Skip to content

Better abstract string concatenation #217

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.

Sign up for GitHub

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

Jump to bottom
Open
TimothyGu opened this issue Apr 30, 2020 · 3 comments · May be fixed by #221
Open

Better abstract string concatenation #217

TimothyGu opened this issue Apr 30, 2020 · 3 comments · May be fixed by #221

Comments

@TimothyGu
Copy link
Member

Due to the way we construct type conversions, oftentimes we see unidiomatic code like

webidl2js/test/__snapshots__/test.js.snap

Line 167 in 7d45dfc

\\"Failed to execute 'bs' on 'BufferSourceTypes': parameter 1\\" + \\" is not of any supported type.\\"
Ideally we want to merge string literals together and even use template literals.

Related to this is #204, but this seems mostly orthogonal and more tractable.
@domenic
Copy link
Member

domenic commented Apr 30, 2020

I wonder if this could be done as a post-processing step using eslint --fix or some more aggressive prettier configuration.

@TimothyGu
Copy link
Member Author

Prettier has a stance of not changing ASTs, so I guess that'd be off. ESLint has no-useless-concat, but sadly no fixer for it. This is easy to do even without a postprocessor though, so maybe we should just write the code.

@ExE-Boss
Copy link
Contributor

ExE-Boss commented May 3, 2020 •
edited
Loading

I already have code for this in https://github.com/ExE-Boss/webidl2js/tree/refactor/precompute-concatenated-strings from a month ago, but I wasn’t sure whether it was something that was needed, and now it needs rebasing.

This was referenced May 3, 2020
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Precompute concatenated strings where possible ExE-Boss/webidl2js
3 participants
@domenic @TimothyGu @ExE-Boss