Group: remove unnecessary complexity for further refactorings

[VorontsovIE]

Hi!
I've done some simplifications which will help in further refactoring. I'm going to introduce grouper objects so that all configuration to be done in constructor. It will prevent scattering grouping logic across entire widget codebase. Grouper object should have two methods: groupName and groupId (to be stored in wo.group_currentGroup). Also groupers are possible to be cached not to extract parameters each time (as it occurs now).
Actually I've done lots of steps in this direction, but I don't want publishing them until you check I didn't removed some unexpected undocumented functionality. I've listed all steps done in the list below.

I hope it will also help me making hierarchical grouping (which is my final purpose, I need it for my work).

Here there are simple changes, some of them just help in code organization, some remove unused options. What was done:

1. Number grouping logic was too difficult. First it calculated an interval boundary and then it amended it by taking maximum of interval boundary and a different value. For ascending order that number was always less than a boundary, for descending order it was a very strange value... I can't figure out why this value was choosen. It came from the very first version of widget and may be it's just an old mistake which doesn't show itself for positive numbers.
   So i just removed this amendment and now it's just an interval boundary. Now the code doesn't rely on group parameter, so it was removed.
   If I'm wrong and this logic is necessary, please let me know, I will revert that change.

2. Language parameter is unused and undocumented, so it was removed. If some grouping logic in future will be language-specific, I think it's better to pass it as an argument to a grouper constructor (I will show it in further commits with grouper object).

3. Now number group name extractor extracts both boundaries. It changes a bit values to be compared dowstream (at line 111), but it doesn't change final result.

4. Row is now obtained from a cache --> normalized --> $row.eq(0). (i.e. not-child row). I guess there are no situations when norm_rows[rowIndex] is falsy, so I replaced currentGroup = norm_rows[rowIndex] ? <extract group name> : currentGroup with currentGroup = <extract group name>.
   May be I don't foresee some cases. Also, please let me know if it's true.

5. Some complex calculations extracted into a separate variables and into a groupHeaderHTML function.

6. currentGroups renamed into collapsedGroups. Initial value given in bindEvent changed from [] to {}.

7. ts.grouping replaced by this because it's shorter and more specific.

As far as I see, these changes haven't broken anything. Widget works just as it did. When you have time, please, check that the changes listed make sense. I'm not sure they should be merged right now, because though it works good, it's still WIP.
p.s. Is it better to push further commits regarding grouping refactoring in the same pull request (so that once you're possible to merge them altogether) or to create separate pull requests?

[VorontsovIE]

Made one more fix:
7) else block in numeric parser has unsatisfiable conditions:

if (num > 1 && txt !== '') {
//...
} else {
  word = (txt + '').match(/\d+/g);
  return word && word.length >= num ? word[num - 1] : txt || '';
} 

is always performed with num == 1 or with txt == ''. So the result is either full numeric match or an empty string. If it is a numeric match, then match is integer numeric.
For now if txt is a float number, it also will be parsed.

If planned functionality (obtain n-th number from a string) is useful, it possibly should be made with another grouper.

[Mottie]

Hi @prijutme4ty!

To be honest, I haven't looked at the grouping code in a while, so I'd have to dig a bit more to figure out why some things were done a particular way.

• The changes to the number group look good; I'll trust what you're saying as the demo still seems to be working fine.
• Yeah, I think I left that language parameter in there by accident. It was replaced by the group_months, group_weeks and group_time. So thanks for cleaning that up!

If you have the motivation, some unit tests would be nice, but since there aren't any in place, I won't force you... the demo looks like it is working fine. I'll have to test it with the pager when I get more free time.

In the mean time, go ahead and push these changes to master - I added you as a collaborator. For big stuff like your last pull request on the main plugin, I would appreciate making those changes in a branch so I can look over it. Also, please go ahead and use grunt test to lint the changes, then grunt if you make any changes to the main widgets, and or core.

[VorontsovIE]

Thank you, @Mottie!
For now grunt test works. Should I update project build with each commit or only after big changes?

I will try not to overuse ability to push changes directly to master. Anyway I want to finalize work on groupers before merging it. I will try to make unit tests too, and sure will let you know when the work is done.

[Mottie]

Oh sorry for not responding.

Well, update the project build when you feel like it needs it, or before pushing to master. I don't have any strict requirements. I think I can trust your judgement on when it needs to be done.

[Mottie]

Hey @prijutme4ty!

I've updated the group widget a few times since this pull request was opened so I'm sure there are a bunch of conflicts. I can incorporate your changes if you feel that the changes are ready.

[VorontsovIE]

Hello, @Mottie. Sorry for a long delay.
I'm not sure yet, about changes. Unfortunately I'm not able to take a thorough look on it till September. In a month I'll have much more relaxed work, and will return to tablesorter.

[Mottie]

I ended up copying some of the changes you have in the refactoring, but I didn't add the groupers method you included... to me, it seems more complex than the original method. If I wanted to add text grouping (already added), all I would need to do is include this code:

$.tablesorter.grouping.types.text = function( config, $column, txt ) {
  return txt;
};

Then use "group-text" in the header.