Add tab option to set tabwidth without turning on indent error checking. #1198

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

This commit fixes issue #1196

Contributor

WolfgangKluge commented Jul 27, 2013

What's the purpose of this option?
Please provide an example.

For example this code from video.min.js has an error on line 9 after id =
id.slice(1) it's missing a semicolon, and the error we get from JSHINT says
character 33 because the tab is expanded to 4 spaces. The patch I sent
fixes that by allowing us to tell JSHINT to count tab characters as 1
character. The eclipse editor expects the character of the error to be
which character counting tabs as one character, so to get it to highlight
the end of line 9 the expected error.character needs to be 20 not 33. I
tried first setting indent to 1 but that turns on indentation checks which
we don't want.

I hope that makes sense why this patch is wanted and in some cases needed.

BR,
Jeremy

(function(window, undefined) {

var document = window.document;
document.createElement("video");
document.createElement("audio");
var VideoJS = function(id, addOptions, ready) {
    var tag;
    if (typeof id == "string") {
        if (id.indexOf("#") === 0) {
            id = id.slice(1)
        }
        if (_V_.players[id]) {
            return _V_.players[id]
        } else {
            tag = _V_.el(id)
        }
    } else {
        tag = id
    }
    if (!tag || !tag.nodeName) {
        throw new TypeError(
                "The element or ID supplied is not valid. (VideoJS)")
    }
    return tag.player || new _V_.Player(tag, addOptions, ready)
}, _V_ = VideoJS, CDN_VERSION = "3.2";
VideoJS.players = {};
VideoJS.options = {
    techOrder : [ "html5", "flash" ],
    html5 : {},
    flash : {
        swf : "http://vjs.zencdn.net/c/video-js.swf"
    },
    width : "auto",
    height : "auto",
    defaultVolume : 0,
    components : {
        posterImage : {},
        textTrackDisplay : {},
        loadingSpinner : {},
        bigPlayButton : {},
        controlBar : {}
    }
};

On Sat, Jul 27, 2013 at 1:09 AM, Wolfgang Kluge notifications@github.comwrote:

What's the purpose of this option?
Please provide an example.


Reply to this email directly or view it on GitHubhttps://github.com/jshint/jshint/pull/1198#issuecomment-21660944
.

Contributor

WolfgangKluge commented Jul 31, 2013

And why do we need an option for it?
Shouldn't tabs always be counted as 1 character (for error reports)?

I may miss the point, but I think this is a greater issue. jshint just can't count tabs easily because they are replaced with n space characters while lexing a source line (inherited from jslint).

Please provide test cases with your pull requests.

Shouldn't tabs always be counted as 1 character (for error reports)?

@WolfgangKluge I believe they should. But in fact, they are counted as 4 characters if "indent": false. Try:

\t1

Replace \t with a literal tab and JSHint will report a missing semicolon at character 6 when it should be character 3 (after the "1").

Now if we set "indent": 1 option, it correctly reports the missing semi-colon at character 3. However, this also outputs an indentation error.

I'm making a Sublime Text plugin and this is a performance killer. It is possible to use "indent": 1 with a custom reporter to filter out indent errors, but obviously, this will output an warning for EVERY indented line - a terrible unnecessarily overhead generated which I have to workaround with even more overhead (having to iterate over the errors array to filter out the indentation errors).

edit: Another workaround, use "indent": 1 and temporarily add //jshint -W015\n at the top of the file before running it through JSHint. Obviously all errors/warnings' line reports will be offset'ed by +1 in comparison to the original file in this case. This will suffice for my use-case for the time being, but obviously this completely disables indentation warnings so it is not a solution for every use case.

Owner

valueof commented Oct 21, 2013

This will be irrelevant soon since we're moving from style checking. indent won't generate style warnings.

@valueof valueof closed this Oct 21, 2013

@antonkovalyov Is the indent option being removed then, since it will no longer produce warnings? Or will it still have any use?

Owner

valueof commented Oct 21, 2013

@UltCombo indent will be used to set indentation level for correct character locations and such.

@jugglinmike jugglinmike referenced this pull request in fb55/domhandler Dec 6, 2013

Closed

Introduce JSHint #5

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