Skip to content

make parse support nest structure#5

Closed
alsotang wants to merge 3 commits intonode-modules:masterfrom
alsotang:master
Closed

make parse support nest structure#5
alsotang wants to merge 3 commits intonode-modules:masterfrom
alsotang:master

Conversation

@alsotang
Copy link
Copy Markdown
Member

var qs = 'x[y][0][v][w]=%CE%ED%BF%D5';
var obj = {'x' : {'y' : [{'v' : {'w' : '雾空'}}]}};
urlencode.parse(qs, {charset: 'gbk'})
  .should.eql(obj);

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 28, 2014

这改动也太大了吧, 加了 nest structure, 要加这么多东西?

@alsotang
Copy link
Copy Markdown
Member Author

我把目录结构拆了下,主要是 lib/parse.js 这个文件是新增内容。

@alsotang
Copy link
Copy Markdown
Member Author

嵌套的 querystring 还会有这种东西的 foo=bar&foo=baz,所以代码少不了。

Comment thread lib/parse.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是干嘛的呢? arr.indexOf 不起作用么?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我修修这些奇怪的地方,这些代码是从 node-querystirng 拷来的。

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我们不是 node-querystring, 没必要去照抄, 原来的代码能work, 没必要去修改.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

原来的代码不是不支持嵌套的结构吗?
On 2014年4月28日, at 14:40, fengmk2 notifications@github.com wrote:

In lib/parse.js:

  • * Object#toString() ref for stringify().
  • /
    +
    +var toString = Object.prototype.toString;
    +
    +/
    *
  • * Object#hasOwnProperty ref
  • /
    +
    +var hasOwnProperty = Object.prototype.hasOwnProperty;
    +
    +/
    *
  • * Array#indexOf shim.
  • */
    +
    +var indexOf = typeof Array.prototype.indexOf === 'function'
    我们不是 node-querystring, 没必要去照抄, 原来的代码能work, 没必要去修改.


Reply to this email directly or view it on GitHub.

@alsotang
Copy link
Copy Markdown
Member Author

我把那些 shim functions 都去掉了。可能是因为 node-querystring 要支持前端所以搞了这些 shim 吧。
我们这个 iconv-lite 去不了前端,所以注定只能在后端用了。

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 29, 2014

我先将代码拉下来本地看看

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 29, 2014

lib/index.js 引用了 lib/parse.js , lib/parse.js 又引用了 lib/index.js ?

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 29, 2014

现在按 node-querystring 怎么感觉那么复杂? 之前我是按 https://github.com/joyent/node/blob/master/lib/querystring.js 来写的, 代码挺简单的.

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 29, 2014

我觉得应该给 https://github.com/visionmedia/node-querystring 提交pr, 让它支持自定义 encode 和 decode 方法, 就ok了, 没必要重现实现一个.

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 29, 2014

@alsotang 我起了一个头, 给 node 的 querystring 提交了一个 pr
你给 tj 的 qs 模块也提交一个 pr 吧

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented Apr 29, 2014

@alsotang �有动静了么?

@alsotang
Copy link
Copy Markdown
Member Author

alsotang commented May 4, 2014

不好意思哈,前几天没动电脑。
那啥,node 原生的 querystring 就是不支持嵌套的,即使是 utf-8 的也不支持。

@alsotang
Copy link
Copy Markdown
Member Author

alsotang commented May 4, 2014

forwardemail/superagent#368

我前两天刚帮 tj 的 superagent 提了个 charset 方面的代码,直接被关闭了。。不知道是不是外国人不愿意趟字符编码的浑水

@alsotang
Copy link
Copy Markdown
Member Author

alsotang commented May 4, 2014

我等等看你那边 querystring 的动静再帮 tj 提吧。官方的 qs 是 tj 的 qs 的一个功能子集,不支持嵌套。

@fengmk2
Copy link
Copy Markdown
Member

fengmk2 commented May 24, 2014

@alsotang node已经合并了,给tj提吧,urlencode就不干这事了

@alsotang
Copy link
Copy Markdown
Member Author

好的。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants