Thanks. Most of them done.

Regards

Tony

I tested the current version from GitHub and found till now only one small problem.

I have one Grid where a column has formatter: 'showlink'. The data send from server have sometime null values. Previous version on jqGrid formated this value like an empty string. The current version from GitHub instead format 'null' value like clickable null. It seems to me that the line 235

shortly added inside of $.fn.fmatter.showlink function in the file jquery.fmatter.js should be removed. At least I don't undestand it's meaning.

Now you undestand, that working with JSLint tooks not so little fime. I hope you will not regret, that begin modifing of jqGrid corresponds of JSLint recomendations.

Inspide of large number of changes in jqGrid I found, that some bugs is not yet changed. For example, the line 160 of grid.custom.js looks the same as before:

but it should be fixed to

One more clear bug, which I described befor (see /blog/?page_id=393/bugs/more-bug-fixes-and-other-suggestions-found-with-respect-of-jslint-tool/) is in the lines 203-204 of grid.jqueryui.js. The code

should be fixed to

The line 19 of grid.subgrid.js:

should be fixed to

and the line 280 of grid.treegrid.js:

should be fixed to

The line 237 of jquery.fmatter.js:

should be also changed to

It's only the main not yet fixed problems described in my previous long post.

Moreover if we stay on grid.base.js and try to make this the most important file more clear, we could reduce JSLint recomendation if we change the order of functions in the grid.base.js file. For example, inside of addCell function (lines 392-397) in the line 394 will be used formatter function defined in the line 398. If one swap the order this two functions in the grid.base.js file one receive less warnings. It seems to me, that for all JavaScript engine and JSLink also it would be easyer to analyse the code after such changes. In one a little change position of the functions formatter, cellVal, updatepager, sortArrayData, beginReq, endReq in the code one receive much fewer warnings.

Function IntNum from the grid.base.js, used inside of grid.base.js only, has first capital character, which should be used for constructors only corresponds to the JavaScript name conversion. The same problem exists only with the function DaysArray from grid.common.js (defined in the line 591). I find the JSLint recomendation good. One can rename IntNum to intNum and DaysArray to daysArray and make all jqGrid function in the same name conversion.

Another small problem exists in lines 967-968:

JSLint comment these line so: Unescaped '-'. It seems to me here we have almost the same problem like missing semicolon. All JavaScript engine which I knows will interpret these lines without any errors, but generally '-' character between '[' and ']' is used for difinition of ranges (like [A-Z]). So one '-' should be escaped to prevent it from being confused with a range hyphen. Of cause here we have no other characters after '-' before ']', so we have not a real bug. Nevetherless the lines

I personally find more readable.

One more LSLint recommendation about lines 196 and 252:

and

I find also OK. The fixed lines

and

will looks more like the rest of the jqGrid code.

These lines probably come in jqGrid after the suggestion from the discussion /blog/?page_id=393/discussion/jqgrid-data-loading-performace/. One could improve these lines later if one bind onselectstart not to document, but to an element (see. http://plugins.jquery.com/file.....1.0.js.txt) or use different technic depend on browser (like

etc. see, for example, http://mediavrog.net/blog/2007.....t-per-css/ or functions disableSelection and enableSelection in lines 117-129 of jquery.ui.core.js).

One more small comment about the JSLint suggestion to the last line of grid.base.js code. How I interpret the recommendation is to replace code

to the code

You can deceide youself whether to follow this recomendation.

Sorry my post will be longer and longer. Better we can discuss other subjects later.

Best regards
Oleg

I have updated a lot of thing. We need to test it in order prevent unwantend behaviour.

Also Have found a lot of mistakes and etc.

Best Regards

Tony

My main goal was to show you, that the usage of JSLint can be really very helpful. So it would be good if you include JSLint in the process of the future release builds of jqGrid.

I also think (like you) that one should be very careful if one makes a lot of changes in a working code. But nevertheless please don’t forget to fix some clear bugs which I described in my two posts: like usage $s or ts instead of $t in grid.custom.js (see /blog/?page_id=393/bugs/more-bug-fixes-and-other-suggestions-found-with-respect-of-jslint-tool/) or missing "var" declarations. (Otherwise I had spend a lot of my time for nothing)

One more remark: I war at the beginning much more careful with changes of the "Equal" and "Not Equal" operators ("==" and "!==") to "Identically Equal" and "Not Identically Equal" ("===" and "!==="), which makes comparison without any type conversion.  Then I make one test. I changed all "Equal" and "Not Equal" operators to "Identically Equal" and "Not Identically Equal" in grid.base.js. Then I tested my current program. Till now I didn’t found any problems! Now I am much more optimistic in the subject.

I see now a trend in JavaScript to usage more strict dialect of the language (see "strict more" in ECMAScript 5, for example, or http://ejohn.org/blog/ecmascri.....-and-more/). I find that jqGrid is now the best Grid plugin for jQuery. But to stay the best jqGrid in the future, you have to use the newest technology and look after (or sometimes follow) the last development trends.

Best regards
Oleg

Thanks again for this. I will use this link to do the changes.

http://www.jslint.com/

Of course we need to make these very carefully and apply them to the next jqGrid release.

Best Regards

Tony

now some suggestions to improve grid.common.js:

Replace the line 234

    var curleft = curtop = 0;

with the line

    var curleft = 0, curtop = 0;

Line 254

    if(options.defaultValue) delete options['defaultValue'];

with the line

    if(options.defaultValue) delete options.defaultValue;

the lines 260-261

    delete opt['id'];
    delete opt['dataInit'];

with the lines

    delete opt.id;
    delete opt.dataInit;

and the line 270

    delete opt['dataEvents'];

with the line

    delete opt.dataEvents;

the line 308

    try {delete options['value'];} catch (e){}

to the line

    try {delete options.value;} catch (e){}

the line 327

    try {delete options['dataUrl'];delete options['value'];} catch (e){}

to the line

    try {delete options.dataUrl;delete options.value;} catch (e){}

the line 332

    delete options['buildSelect'];

to

    delete options.buildSelect;

In the line 341 are missing semicolon. One should replace

    ovm = jQuery.map(ovm,function(n){return jQuery.trim(n)});

to

    ovm = jQuery.map(ovm,function(n){return jQuery.trim(n);});

Because the same reason one should replace

    ovm = jQuery.map(ovm,function(n){return jQuery.trim(n)});

to

    ovm = jQuery.map(ovm,function(n){return jQuery.trim(n);});

line 374. Don't make functions within a loop.

In the line 386 are used variable ov

    ov = document.createElement("option");

which is defined out of scope (see line 370). So it would be better insert for example in the line 360

    var i;

definition of ov variable

    var i, ov;

and remove it from the line 370

    var oSv = options.value, ov;

Lines 386-391 (for-body) should be placed inside of the body of the following if-block

    If (oSv.hasOwnProperty(key)) { }

to filter unwanted properties from the prototype.

The line 446

    for( i =0, len=g.p.colModel.length;i<len; i++){

has len variable, which is not defined. So one could modify the line 444

    var edtrul,i, nm;

to

    var edtrul,i, nm, len;

In the line 546-547 are missing semicolon.

    if(yln.indexOf("m") != -1) {mln=i}
    if(yln.indexOf("d") != -1) {dln=i}

So one should replace these lines to the following

    if(yln.indexOf("m") != -1) {mln=I;}
    if(yln.indexOf("d") != -1) {dln=I;}

Moreover in the line 374

    sv[1] = jQuery.map(sv,function(n,i){if(i>0){return n;}}).join(":")

the function “function(n,i)” is defined within a loop (see line 371). If would be better to define it outside the loop and use it only in the line 371. The corresponding change could be following. We insert a new line which looks like

    var myMapFunc = function(n,i){if(i>0){return n;}};

(please, verify what should this function returns if i=0) after the line 370 and change line 374 to

    sv[1] = jQuery.map(sv, myMapFunc).join(":");

Best regards
Oleg

with the line "if (p.gridComplete = complete) {" I was not sure myself.

If you want I could check other jqGrid-files (not only grid.base.js) when I found a little time for it.

Regards
Oleg

First of all thank you for this.

We have missed this checking since 3.5 version.

The most of your suggestions are allready done. Will be published soon.

Just one note

Line 336

    if (p.gridComplete = complete) {

this is a valid instead and can be rewritten like this

p.gridComplete = complete;

if(p.gridComplete) {

It is written so to save the place

your fix will break the code.

Again thanks

Best Regards

Tony