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

Control of semicolon behaviour #20

Open
eddiesholl opened this issue Jun 19, 2017 · 5 comments · May be fixed by #73
Open

Control of semicolon behaviour #20

eddiesholl opened this issue Jun 19, 2017 · 5 comments · May be fixed by #73

Comments

@eddiesholl
Copy link

eddiesholl commented Jun 19, 2017

Motivation

I'm using astring as part of an atom plugin to help with code generation. It would be great to be able to control whether semicolons are emitted, so that I can match the current style directives of the current project. Right now, semicolons are implemented as a whole bunch of state.write(';') instances.

It would be awesome to have a config option to control semicolon emission. I'm thinking that you could replace all those writes with a semantic state.terminate(), which can check if it should print a semicolon or not.

The only alternative to control this is to basically fork the entire baseGenerator, so you can remove all those writes.

@davidbonnet davidbonnet added this to the 2.0: Custom formatting support milestone Jun 22, 2017
@davidbonnet
Copy link
Owner

davidbonnet commented Jun 23, 2017

This could be handled through a custom output stream, that removes semicolons:

const output = {
  code: '',
  write(code) {
    switch (code) {
      case ';':
        break
      case ');':
        this.code += ')'
        break
      case 'debugger;':
        this.code += 'debugger'
        break
      default:
        this.code += code
        break
    }
  },
}

const { code } = astring.generate(ast, { output })

console.log('Code without semicolons:', code)

There are however two known issues to this:

  1. Not all semicolons can be removed, in particular, when the next expression starts with a parenthesis.
  2. Using a custom output stream has some performance hits.

Thus, a more precise handling is required. This could be a first step towards version 2. Pull requests are welcome.

@Mouradif
Copy link

Mouradif commented Jul 4, 2019

+1 ! I was just about to go for "The only alternative to control this" according to @eddiesholl before checking the issues here.

I will submit a PR right after

Mouradif pushed a commit to Mouradif/astring that referenced this issue Jul 8, 2019
Mouradif pushed a commit to Mouradif/astring that referenced this issue Jul 8, 2019
@KFlash
Copy link

KFlash commented Sep 22, 2020

I looked at this issue and it isn't super hard to implement. For example use an extra function param on statement level. noSemi.

Then in BlockStatement and Program within the iteration loop set the param value to either true or false. Depends if the element is the last one in that array.

Before looping you always set the value to false.

Then you can print semi only if noSemi is falsy.

Don't forget the IfStatement edge case!

Regarding parents. For example as in this example this will be smashed into one, but still valid JS.

{
 (Foo)
Bar
} `

@KFlash
Copy link

KFlash commented Sep 24, 2020

Quick follow up. If you want an option to enable the semis it's possible if you avoid printing the semicolon if .!Opt.Semi || noSemi.

Also be aware that noSemi has to be falsy before entering any blocks.

@Mouradif
Copy link

Mouradif commented Oct 1, 2020

@KFlash did you check my PR above ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants