Little refactoring, moving "general" and "isolated" functions from base ...

[bouks]

...to common

[bouks]

These functions are not directly linked to the grid. They're utils function.

[flack]

Haven't looked at the changes really, but just to be sure: grid.common.js is in the current logic the common functions for the different editing modes (it should probably better be called grid.editing.common.js or something like that) and is listed under the "Editing" headline in the download builder. Are we sure this is the right place for utils functions?

[bouks]

I don't know, but sure the name is only "common". There is also modal functions that are not only for editing. Like [show|hide|close]Modal, findPos, createModal...

[flack]

Yes, @OlegKi mentioned something similar in another discussion. The current structure is not very good, and should be changed. We should just make sure that base does not have any dependencies on other files. Or at the very least, we should mention the dependencies in the download builder.

[OlegKi]

Sorry, @bouks , but I have to disagree with your suggestions again. The grid.common.js file contains the methods used mostly in grid.formedit.js, grid.inlinedit.js and grid.celledit.js (like mentioned above @flack ). One can use only grid.base.js module to fill and display the simple grid. One need additionally jquery.fmatter.js if one use predefined formatters. So the grid.base.js module is already the "common" module in your definition.

I want to repeat one common remark which I wrote before: if one suggest some changes in the code one should write the reason of the changes. For example one can move some methods, which are used very seldom, in separate module. One can make changes, which improve the performance, compatibility, fix some big, add new feature or some other reason. So before you suggest some changes, befor you implement there or before you post new pull request you should have clear answer on the different "why" questions. Look in the list of changes which I do. I hope that you will find alway the reason of the changes in the title or in the short description of the chages.

[bouks]

@OlegKi "common" name in every project is used to put things that are common to "everything", it's a standard. Therefore it should be included every time. But, if you want a read-only distribution, it should be the common's editing functions that must go in another file.

We need to decoupling code.

[bouks]

@OlegKi , there is development files and distribution files. A common file can be separate in dev and merged in dist. "Normal" user should use distribution file. If he use development files, he is an advanced user and should know what he is doing.

[meh-uk]

I agree that having separate files for development is fine.

[OlegKi]

Sorry @bouks , but you still don't answered on the questions why: why you suggest this? What the goal of such changes? Which advantages have the changes?

The more files (modules) we have the more dependencies will exist. Currently the grid.base.js contains either the methods without which no grid can be created (because there used in constructor for example) or the methods which will be used really frequently (like getRowData, getCell showCol and so on). If you would move some methods like $.jgrid.htmlEncode in separate files then we will have one more dependency: grid.base.js depends from new module, but all modules will still depend from grid.base.js and the new module. So I really see no advantage.

I repeat one more: first should be the description of the goals of the changes and only later then the corresponding implementation in form of the pull request. The description "Little refactoring, moving "general" and "isolated" functions from base to common" just describes what you do, but not why it have sense.

In my opinion the grid.base.js is the last module where one should start with refactoring. The first candidate is grid.custom.js and `in my opinion: what Tony did. The next will begrid.formedit.jsand navigator part ofgrid.inlinedit.js. Thengrid.common.js`, ....

I would eliminate the usage of JsonXml.js, rewrite grid.tbltogrid.js and making tableToGrid not more global, ... There are a lot of things to do.

[bouks]

@OlegKi

I'd like not to full explain some obvious things, and repeat the same things every time, i have no time to waste. I'd like you to read what i wrote here and in emails, All is there ! flack objected some things but he understand the "why", and we can then discuss and advance.

[meh-uk]

Everyone ok to merge this?

[flack]

-1 from me. As discussed in email, grid.common.js is currently not what the name suggests, so all the PR does is create a cyclical dependency between base and the editing module

[bouks]

Are modal's functions only for editing ? Are they not used also for warning or information messages ?

If so, we could to determine what functions are essentially for editing and move them to editing (.common file ?) and let the others in common (only) file.

I think, in the future and in a (generic) common file, we could move some parts of existing functions to make these parts independant, with no dependencies and reusable.