CDATA filtering options, new item addition functions, unit tests, bug fixes #38

rv-kip · 2014-12-20T01:50:58Z

Optional disabling of CDATA wrapping by field name
Adding items by concatenation of existing items or by complete replacement of existing items
Associated unit tests
Runnable example code examples/simple.js
Corresponding readme updates.

This PR supercedes #37, which has some relevant discussion.
This PR is similar to existing PR #28, but allows for finer control over what fields can be skipped for CDATA wrapping.

…concat_items and replace_items. Associated unit tests

…. Adding test for XML escaping when no_cdata_fields is used

… by item() resulting in inconsistencies like missing category field. Added test for this issue

…ply-for-new-pr Conflicts: lib/index.js test/rss.test.js

dylang · 2014-12-20T01:55:50Z

package.json

@@ -57,10 +58,10 @@
  "dependencies": {
    "folderify": "^0.6.0",
    "mime-types": "^2.0.3",
+    "mime": "^1.2.11",    


mime-types should be good enough

When I clone (or rm -rf node_modules), npm i, and npm test, I get an error for missing mime module unless mime is in package.json

see above :)

rv-kip · 2014-12-21T18:11:55Z

This is ready for review.

rv-kip · 2015-01-12T19:15:10Z

(bump)

rv-kip · 2015-01-20T19:37:48Z

@dylang, Is there any more info, documentation, or testing you need from me for this PR?

dylang · 2015-01-21T05:07:22Z

Sorry for taking so long with this. I'll try to get to it later this week.

rv-kip · 2015-01-21T19:41:13Z

Not a problem. Just wanted to make sure you weren't waiting for something from me.

dylang · 2015-02-08T21:38:04Z

The cdata work looks good. I don't like the generic output function name, but at least it's short.

The replace_items and concat_items feel like they should be in a separate PR. Since you've already done the work here there's no need to create a separate PR.

I think a better API would be something like feed.removeItems() and feed.addItems(<array of items>). What do you think?

rv-kip · 2015-02-09T23:19:12Z

@dylang Your function rename suggestions are fine. Better to change them now than have a legacy support issue.

rv-kip · 2015-02-09T23:28:42Z

To clarify, should feed.removeItems() remove all existing items?

Updating function names, docs, and unit tests as per discussion

rv-kip · 2015-02-12T23:13:08Z

@dylang, my latest commit changes the names of the extra functions and adds a unit test for removeAllItems() Ready for review.

rv-kip · 2015-03-16T21:03:25Z

@dylang, How are we going on this?

rv-kip · 2015-04-27T21:13:19Z

@dylang... Any progress on this?

rv-kip · 2015-05-29T18:43:51Z

@dylang, I've released the rss-braider module that depends on the changes in this PR. I'd prefer to be dependent on your repo and not my fork. Let me know if there's anything I can do to help with merging this PR. 😄
https://www.npmjs.com/package/rss-braider

rv-kip · 2018-02-27T20:11:43Z

It seems like the Travis failure is related to npm3. This env has an npm config of

...
  "before_install": "npm -g i npm@3",
...

rv-kip · 2018-06-27T17:10:19Z

close/open to trigger Travis

rajinikumar · 2018-07-16T11:16:32Z

@dylang, Could you please reivew the @rv-kip changes.

rajinikumar · 2018-07-16T11:20:42Z

@dylang Please review @rv-kip changes. because he done the changes few years back.

mattymess · 2019-10-10T22:28:37Z

Any movement on this? Would love to see it get merged

Kip Gebhardt and others added 19 commits December 12, 2014 16:14

Merge from upstream master

1fab44d

Allowing selective disabling of CDATA wrapping by field name. Adding …

23aa233

…concat_items and replace_items. Associated unit tests

Update readme.md

ab2ab66

Everyone loves an example

87e0c77

Didn't end up using underscore.

6ac9af9

Changing param name to no_encode_fields in all needed places

3070e49

... plus one more place

f0fc81f

Going back to 'no_cdata_fields' for name. Clarifying use case in docs…

6fba453

…. Adding test for XML escaping when no_cdata_fields is used

Fixing typo in example

38363d1

README cleanup for custom_namespaces and custom_elements

e23f8f6

Fixing bug. Items passed in on RSS obj creation did not get processed…

f528b84

… by item() resulting in inconsistencies like missing category field. Added test for this issue

Merge branch 'master' of https://github.com/dylang/node-rss into reap…

ff24ee9

…ply-for-new-pr Conflicts: lib/index.js test/rss.test.js

Moving tests from defunct PR into new branch

ce46bbf

Added tests for new functionality

5993cd5

Missing mime dependency and a dupe dep

6478f12

Removing old mocha test

65a9d42

Documentation update and readme generation

fa4d015

Moving mime to dependencies

5450f8d

Indenting change on example. Commenting.

c1f6460

dylang reviewed Dec 20, 2014
View reviewed changes

Removing unecessary mime module.

603b252

dylang mentioned this pull request Feb 8, 2015

Option to specify CDATA wrapper for content #28

Closed

Kip Gebhardt and others added 2 commits February 10, 2015 14:25

Updating function names, docs, and unit tests as per discussion

ddfa350

Merge pull request #1 from rv-kip/function_renaming

86f2a74

Updating function names, docs, and unit tests as per discussion

Merge branch 'master' of https://github.com/dylang/node-rss

706e137

Merge branch 'master' of https://github.com/dylang/node-rss

8d14204

rv-kip and others added 4 commits November 2, 2015 08:24

Fixing merge conflicts

bfe54b7

Removing newlines that were breaking unit tests after 87c956d

cd123b6

Merge branch 'master' into master

a8cfc9e

Update package.json

6b7e4d7

rv-kip closed this Feb 27, 2018

rv-kip reopened this Feb 27, 2018

rv-kip mentioned this pull request Feb 27, 2018

no_cdata_fields not working rv-kip/rss-braider#7

Open

rv-kip closed this Jun 27, 2018

rv-kip reopened this Jun 27, 2018

dylang requested a review from ErisDS July 16, 2018 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CDATA filtering options, new item addition functions, unit tests, bug fixes #38

CDATA filtering options, new item addition functions, unit tests, bug fixes #38

rv-kip commented Dec 20, 2014

dylang Dec 20, 2014

rv-kip Dec 20, 2014

dylang Dec 20, 2014

rv-kip commented Dec 21, 2014

rv-kip commented Jan 12, 2015

rv-kip commented Jan 20, 2015

dylang commented Jan 21, 2015

rv-kip commented Jan 21, 2015

dylang commented Feb 8, 2015

rv-kip commented Feb 9, 2015

rv-kip commented Feb 9, 2015

rv-kip commented Feb 12, 2015

rv-kip commented Mar 16, 2015

rv-kip commented Apr 27, 2015

rv-kip commented May 29, 2015

rv-kip commented Feb 27, 2018

rv-kip commented Jun 27, 2018

rajinikumar commented Jul 16, 2018

rajinikumar commented Jul 16, 2018

mattymess commented Oct 10, 2019

CDATA filtering options, new item addition functions, unit tests, bug fixes #38

Are you sure you want to change the base?

CDATA filtering options, new item addition functions, unit tests, bug fixes #38

Conversation

rv-kip commented Dec 20, 2014

dylang Dec 20, 2014

Choose a reason for hiding this comment

rv-kip Dec 20, 2014

Choose a reason for hiding this comment

dylang Dec 20, 2014

Choose a reason for hiding this comment

rv-kip commented Dec 21, 2014

rv-kip commented Jan 12, 2015

rv-kip commented Jan 20, 2015

dylang commented Jan 21, 2015

rv-kip commented Jan 21, 2015

dylang commented Feb 8, 2015

rv-kip commented Feb 9, 2015

rv-kip commented Feb 9, 2015

rv-kip commented Feb 12, 2015

rv-kip commented Mar 16, 2015

rv-kip commented Apr 27, 2015

rv-kip commented May 29, 2015

rv-kip commented Feb 27, 2018

rv-kip commented Jun 27, 2018

rajinikumar commented Jul 16, 2018

rajinikumar commented Jul 16, 2018

mattymess commented Oct 10, 2019