vkadam / grunt-jsbeautifier

Beautify js, css, html and json files using Grunt and https://github.com/einars/js-beautify
MIT License
119 stars 28 forks source link

maxPreserveNewlines doesn't seem to do anything #32

Closed corysimmons closed 10 years ago

corysimmons commented 10 years ago

I have the following Gruntfile.coffee:

module.exports = (grunt) ->
  grunt.initConfig

    app:
      cssFileTypes: [
        './app/assets/stylesheets/**/*.css'
      ],
      jsFileTypes: [
        './app/assets/javascripts/**/*.js',
        './app/assets/javascripts/**/*.erb'
      ],
      coffeeFileTypes: [
        './app/assets/javascripts/**/*.coffee'
      ],
      htmlFileTypes: [
        './app/views/**/*.html',
        './app/views/**/*.erb',
        './app/views/**/*.haml',
        './app/views/**/*.hamlc'
      ]

    jsbeautifier:
      files: [
        '<%= app.jsFileTypes %>',
        '<%= app.cssFileTypes %>',
        './app/views/**/*.html'
      ]
      options:
        js:
          fileTypes: [
            '.js.erb'
          ]
          indentSize: 2
          maxPreserveNewlines: 1
        css:
          indentSize: 2
          maxPreserveNewlines: 1
        html:
          fileTypes: [
            '.erb',
            '.html.erb',
            '.haml',
            '.html.haml',
            '.hamlc',
            '.html.hamlc'
          ]
          indentSize: 2
          maxPreserveNewlines: 1
          braceStyle: 'end-expand'
          unformatted: [
            'inline',
            '%'
          ]

    copy:
      inplace:
        files: [
          expand: true
          src: [
            '<%= app.jsFileTypes %>',
            '<%= app.cssFileTypes %>',
            '<%= app.htmlFileTypes %>'
          ]
        ]
        options:
          process: (content, filepath) ->
            # Remove excess lines from end of file
            if /\n+$/.test(content)
              content = content.replace(/\n+$/, '')
            # Remove trailing whitespace
            if /\s+$/gm.test(content)
              content = content.replace(/\s+$/gm, '')
            # Replace tabs with 2 spaces
            if /\t/gm.test(content)
              content = content.replace(/\t/gm, '  ')
            # Add newline at end of file
            content + '\n'

    lintspaces:
      all:
        src: [
          '<%= app.jsFileTypes %>',
          '<%= app.cssFileTypes %>',
          '<%= app.htmlFileTypes %>'
        ]
        options:
          newline: true
          trailingspaces: true
          indentation: 'spaces'
          spaces: 2
          ignores: [
            'js-comments',
            'html-comments',
            'ruby-comments'
          ]

    jshint:
      all: ['<%= app.jsFileTypes %>']

    coffeelint:
      app: ['<%= app.coffeeFileTypes %>'],
      options:
        'max_line_length':
          'level': 'ignore'

    watch:
      files: [
        '<%= app.jsFileTypes %>',
        '<%= app.cssFileTypes %>',
        '<%= app.htmlFileTypes %>'
      ]
      options:
        atBegin: true
        debounceDelay: 5000
      tasks: [
        'jsbeautifier',
        'copy',
        'lintspaces'
      ]

  require('matchdep').filterDev('grunt-*').forEach grunt.loadNpmTasks
  grunt.registerTask 'default', ['watch']

I don't think maxPreserveNewlines is working for any of the file types. I'm expecting it to leave one line between blocks of code where someone else left a bunch of lines first. For instance:

foo.html.erb

<p>

Hello

</p>

Would turn into..

<p>

Hello

</p>

I'd love any help you can offer.

vkadam commented 10 years ago

I have seen this issue in past. There are couple of issue already added for this on js-beautify, https://github.com/einars/js-beautify/issues?state=open

For now workaround is use

maxPreserveNewlines: 2

instead of

maxPreserveNewlines: 1
corysimmons commented 10 years ago

Even maxPreserveNewlines: 2 does nothing to help the problem. :(

vkadam commented 10 years ago

Also try adding

preserve_newlines: true
vkadam commented 10 years ago

Here are my findings, For js files specifying preserve_newlines attribute is not mandatory, is maxPreserveNewlines is specified preserve_newlines is made true. But for html files, is mandatory to specify, preserve_newlines.

I have added test cases for it, https://github.com/vkadam/grunt-jsbeautifier/commit/accb24159b6d6e4b8afc76e2eed624006cc5d85b

corysimmons commented 10 years ago

Ah, I think that explains it. How weird....

corysimmons commented 10 years ago

Should we file an issue with JS Beautify then? Seems unexpected.

vkadam commented 10 years ago

I will file a issue for it....

corysimmons commented 10 years ago

Nevermind, it was the "Remove trailing whitespace" thing I did. I suck. Sorry for wasting your time. :)

corysimmons commented 10 years ago

Per your tests, and for others looking at this: preserveNewLines instead of preserve_newlines or preserveNewlines (although this goes with maxPreserveNewlines).

vkadam commented 10 years ago

Yeap, you are right. This is for making Gruntfile.js jshint/lint happy. See this issue https://github.com/vkadam/grunt-jsbeautifier/issues/17 and commit https://github.com/vkadam/grunt-jsbeautifier/commit/ee4ed28783fe28e4fb8ab94a9f1e62f64a34fbc4