src,url: separate some tables out of node_url.cc

[XadillaX]

The purpose of separating is for readability and maintainability.

[addaleax]

Suggested change

	externconstchar*hex[256];
	externconstchar*consthex[256];

(Aside -- using a single string of '%00\0%01\0%02\0...' and then accessing it as hex[index * 4] saves a bit more space and probably improves cache locality a bit.)

[XadillaX]

I've changed it to a single string. But to be honest, I think the readability isn't increased much.

[addaleax]

Right – I didn’t say it would increase readability :)

[jasnell]

As an alternative to a new *.cc file, these could be moved into a node_url-inl.h header

[XadillaX]

As an alternative to a new *.cc file, these could be moved into a node_url-inl.h header

Because it's real data, not only declaration. If node_url-inl.h is mis-included by another file, the compilation will failed. So I think it's better to be stored in a *.cc file.

[RaisinTen]

As an alternative to a new *.cc file, these could be moved into a node_url-inl.h header

Because it's real data, not only declaration. If node_url-inl.h is mis-included by another file, the compilation will failed. So I think it's better to be stored in a *.cc file.

Should we add a TODO comment to move the variables to node_url-inl.h when #38807 lands because in C++17 we can inline variables?

[addaleax]

As an alternative to a new *.cc file, these could be moved into a node_url-inl.h header

Because it's real data, not only declaration. If node_url-inl.h is mis-included by another file, the compilation will failed. So I think it's better to be stored in a *.cc file.

Should we add a TODO comment to move the variables to node_url-inl.h when #38807 lands because in C++17 we can inline variables?

All we’d get out of that is probably just longer compilation times. @XadillaX is right, this is actual data that is accessed as data, so inlining has no benefit here and putting it in a .cc file is just the right thing to do, and I don’t understand why something else would be suggested here.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/38644/

[XadillaX]

Landed in 38a15d8