Lots of merges and enhancements

[ProLoser]

I tried to merge in just about every outstanding fork, but gave up on @jcdarwin cuz he just refactored the entire engine and I don't have THAT much energy.

Also, although I merged @dnakov in also, I didn't keep the changes from 4dae463d7641ccb42bc9c7d10db492561493d880 because I am not sure if I trust them. I would be curious to know more about this bug before re-applying it

[ProLoser]

Oh yeah, to explain some stuff:

• I lumped together the prepend, append, before, after and replace logic since it's identical
• I changed the order to put non-destructive ops near the top and the most destructive ops closer to the bottom. This way people won't be confused when they are trying to select/traverse DOM that has changed significantly. It is now:
  1. read
  2. prefix
  3. suffix
  4. text
  5. update
  6. prepend
  7. append
  8. before
  9. after
  10. replace
  11. remove
  12. callback

[ProLoser]

Huzzah! I broke everything apparently... god dammit.

[ProLoser]

Okay fixed my errors, I forgot to accomodate for the path option. However after reviewing your expected/order.html I realized what you had in mind when you put read first and then remove. For me, I had been dealing with other scenarios, like injecting the new DOM in/around where the old DOM was, and then removing the old DOM afterwards. I can understand your use-case however and am now no longer sure what order is best, yours or mine.

[cgross]

Phew! Thats alot of changes! In the future, it would be easier to deal with this many changes as separate PRs.

Lots of thoughts on these changes. First though, this is a big PR, adds multiple new features, and there are zero new tests. The existing tests will help cover existing features, but w/o any tests for these new features and changes, I'm really nervous about merging this.

Going through the significant changes one by one:

Filter function on read This one looks simple enough. But having a test will ensure that this works as expected now and more importantly, a test will ensure that this feature doesnt get broken by some later change. Also, the code that applies a filter function of function(val) { return val; } is changing behavior. Now any val that is a falsy value will be filtered. Maybe thats desirable. I'm not sure. But to ensure we don't change behavior it should be function(val) { return true; }.

Multiple dests As I said on the original PR, I think this is a great addition but it needs a test or two.

Option for silent output Looks good but I'd probably still print out the messages if grunt is in verbose mode. I would alter it so that there's a logger that is set to grunt.log if !options.silent and is set to grunt.verbose if options.silent === true.

xml mode Just tests here.

Replacing &apos A global replace? What if someone has a text block where they want "&apos"? I'm not sure thats safe. Cheerio replacing the single quotes with the escape code is annoying but I trust that they're doing it in a way that doesn't actually change the resulting parsed HTML. Doing a simple global replace does not seem safe.

Read callback option This is definitely a map function. Map is the term used for iterating over an array and modifying the values. You can almost do vals = vals.map(options.map) if you change callback to map. Also, should this happen before or after the isPath logic is applied? Probably doesn't matter too much but I'd think after.

Order Yea there are really competing desires for how to order things. I think the primary use-case that I'd like to support with dom_munger is: read script/link tags -> concat/min the js and css -> remove the script/link tags -> add the new script/link tags with the new concat/min versions. So I'd like to keep the order that way. I think your use-case does help give evidence that it would be really nice to have a way to specify the order manually. I've been thinking about this and I think the best solution would be to allow the operations to be specified as an array (and you can order the array however you want). So instead of this:

options :{
   read: {...},
   append:{...}
}

it could be this:

options: {
   actions: [
      {type:'read', ...},
      {type:'append',...}
   ]
}

IMO thats the optimum solution. If you feel like tackling that great! Otherwise, we can keep the order as is until I get around to tackling it.

[ProLoser]

I'm aware separate PR's would be better, but you didn't seem to be doing anything about those other PR's lol.

Yes. I need to make more tests. Tests in this approach are annoyingly tedious.

I agree about the &apos code and removed it. I also changed the filtering behavior. It now performs both mapping AND filtering. This means you can both modify the values used OR you can return a falsey value to exclude the value completely.

I also agree that where the isPath logic is executed changes behaviors (such as before or after the map/filter).

I may rip this into pieces, but really I just had some free time on my hands when i wrote this and we'll have to see if I find that time again.