Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

disallow trailing whitespace on empty lines #1554

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

We've flip-flopped on this issue. Recently in #182 @antonkovalyov expressed interest in a patch disallowing (again) trailing whitespace on otherwise empty lines. I'm not sure whether this breaking change is acceptable. Ugly as it might be, we could appease both sides by supporting three values for trailing: false, true, and "strict". At this stage I'm submitting the simplest possible change. I will update the PR in response to feedback.

okv commented Mar 20, 2014

it would be great if this changes will be merged into master

My solution is to run jshint first, then do a couple of whitespace tests using grep:

FIND_JS_FILES = find . -name '*.js' -not -path './coverage/*' -not -path './node_modules/*' -print0

.PHONY: lint
lint:
    $(FIND_JS_FILES) | xargs -0 $(JSHINT)
    $(FIND_JS_FILES) | GREP_COLOR=41 xargs -0 grep --color '<replace-with-literal-tab-character>' >&2 ; \
        if [ $$? = 0 ] ; then echo Error: Tab character found. >&2 ; exit 1 ; fi
    $(FIND_JS_FILES) | GREP_COLOR=41 xargs -0 grep --color ' \+$$' >&2 ; \
        if [ $$? = 0 ] ; then echo Error: Trailing whitespace. >&2 ; exit 1 ; fi

This displays tab characters and trailing spaces in red, and ensures that make exits with the appropriate error code if matches are found.

@valueof valueof closed this in 0c0e193 Mar 27, 2014

Just to clarify, @antonkovalyov, this means JSHint will no longer take any stance on trailing whitespace, correct?

Owner

valueof commented Mar 27, 2014

Correct. JSHint is moving away from purely stylistic warnings. You should use JSCS for that.

On Mar 27, 2014, at 11:08 AM, David Chambers notifications@github.com wrote:

Just to clarify, @antonkovalyov, this means JSHint will no longer take any stance on trailing whitespace, correct?


Reply to this email directly or view it on GitHub.

Sounds good. Thanks for the clarification.

@davidchambers davidchambers deleted the davidchambers:trailing-whitespace branch Mar 27, 2014

@mduan mduan referenced this pull request in UWFlow/rmc Apr 18, 2014

Closed

Add whitespace linting for all files #106

jugglinmike added a commit to jugglinmike/jshint that referenced this pull request Oct 21, 2014

myabc added a commit to opf/openproject that referenced this pull request Nov 14, 2014

Remove deleted trailing option from JSHint config
Functionality removed from JSHint.
See: jshint/jshint#1554

Signed-off-by: Alex Coles <alex@alexbcoles.com>

@Tidal-Wave Tidal-Wave referenced this pull request in roots/sage Jan 23, 2015

Closed

'trailing' no longer an option in jshint #1275

austinpray added a commit to roots/sage that referenced this pull request Jan 23, 2015

@austinpray austinpray referenced this pull request in roots/sage Jan 23, 2015

Merged

remove trailing option #1276

woodb pushed a commit to myfossil/bbPress that referenced this pull request Jan 24, 2015

Remove `trailing` option from `.jshintrc`, as of JSHint v2.5 `trailin…
…g` is no longer an option.

See jshint/jshint#1554
Fixes #2746

git-svn-id: https://bbpress.svn.wordpress.org/trunk@5600 9866e705-20ec-0310-96e7-cbb4277adcfb

ntwb added a commit to ntwb/bbPress that referenced this pull request Jan 31, 2015

Remove `trailing` option from `.jshintrc`, as of JSHint v2.5 `trailin…
…g` is no longer an option.

See jshint/jshint#1554
Fixes #2746

git-svn-id: https://bbpress.svn.wordpress.org/trunk@5600 9866e705-20ec-0310-96e7-cbb4277adcfb

ow pushed a commit to ow/popsicle-ice that referenced this pull request Jun 30, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment