peggyjs / peggy

Peggy: Parser generator for JavaScript
https://peggyjs.org/
MIT License
914 stars 65 forks source link

Proposal to improve readability of generate-js.js #391

Closed nene closed 1 year ago

nene commented 1 year ago

Currently the JavaScript code generation contains large blobs of code in the following manner:

parts.push(
  "(function(root, factory) {",
  "  if (typeof define === \"function\" && define.amd) {",
  "    define(" + dependencies + ", factory);",
  "  } else if (typeof module === \"object\" && module.exports) {",
  "    module.exports = factory(" + requires + ");"
);

I find it makes it both hard to read such code and also to modify it. For example: I look at the generated parser code, modify it a little, and would like to copy-paste it back to the code-generator, but that's really inconvenient to do.

I would propose the use of template strings:

parts.push(`
  (function(root, factory) {
    if (typeof define === "function" && define.amd) {
      define(${dependencies}, factory);
    } else if (typeof module === "object" && module.exports) {
      module.exports = factory(${requires});
`);

The above approach would however include extra unwanted indentation at the start of each line. I have personally used a micro-library dedent-js to eliminate this shortcoming:

parts.push(dedent`
  (function(root, factory) {
    if (typeof define === "function" && define.amd) {
      define(${dependencies}, factory);
    } else if (typeof module === "object" && module.exports) {
      module.exports = factory(${requires});
`);

now the result would be the same as writing:

parts.push(
`(function(root, factory) {
  if (typeof define === "function" && define.amd) {
    define(${dependencies}, factory);
  } else if (typeof module === "object" && module.exports) {
    module.exports = factory(${requires});`
);

I wonder whether there's any actual reason for the parts array to contain an array of individual lines? I don't think there is, but even if it's a requirement, one can write an utility that splits a template string into lines.

Some context:

I've been looking into making ts-pegjs support the latest Peggy 3.0. But the code generation in Peggy has changed quite a lot since 2.0, so I will likely need to do a complete rewrite of the code generation in ts-pegjs. But working with these quoted code blocks is quite annoying. I could simply adopt the template-string based approach in ts-pegs, but I'd also like to keep the code as similar to Peggy as possible to ease syncing with changes in the future.

hildjj commented 1 year ago

Here are some other thoughts on this: https://github.com/peggyjs/peggy/discussions/378

nene commented 1 year ago

Thanks. I did read that a few weeks ago. Unfortunately I'm not familiar enough with the internals of Peggy to have any sort of informed opinion on this larger code-generation revamp.

reverofevil commented 1 year ago

@nene The "proper" way to generate code is to use something like @babel/template. Code is not a string, and should not be generated by string concatenation.

hildjj commented 1 year ago

I'm going to close this in favor of #378 until we come up with an approach. Re-open if you strongly disagree.