Purge the term 'vancode'

Subscribe lol

Tagging.

Starting patch. I couldn't come up with a good name. So i added some info saying its a drupalism and a link to the issue that introduced it.

crosspost

The patch is definitely an improvement on the current situation!

(I do wonder whether we could replace the system with something better though...)

+++ b/modules/comment/comment.moduleundefined
@@ -2455,7 +2456,7 @@
+function comment_int2vancode($i = 0) {

The substitution of "2" for "to" is a bit un-Drupaly. I'd suggest:
comment_int_to_alphadecimal()
and
comment_alphadecimal_to_int().
Then we can kill the comment explaining wtf a vancode is.

+++ b/modules/comment/comment.moduleundefined
@@ -2447,7 +2447,8 @@
  * with a numerical value in base 36. Vancodes can be sorted as strings
- * without messing up numerical order.

"These alphadecimal codes can be sorted as strings without altering numerical order."

+++ b/modules/comment/comment.moduleundefined
@@ -1525,7 +1525,7 @@
+          $thread = $parent->thread . '.' . comment_int2vancode(comment_vancode2int($last) + 1) . '/';

This is confusing. While we're cleaning it up, can we add comment_increment_alphadecimal() as a human-readable wrapper for this?

People still would google for alphadecimal and feeling unlucky

I dunno, it got me http://en.wikipedia.org/wiki/Base_36 as the top result :D

Maybe add a few lines of explanation in the docs for comment_int_to_alphadecimal()?

Well it is base 36 with a digit in front of it...

The word "alphadecimal" is an actual word, and lots of people know what it means. The word "vancode" is made up. :)

"Consists of a leading character indicating length, followed by N digits in base 36 (alphadecimal). These codes can be sorted as strings without altering numerical order."

And then put an @see to this function in the docblock of every function that calls it.

There's also a reference we missed in comment.install.

Oh wow.

I fail at M%.

Ok what about this patch?

#17: Did you see #15 ? :)

ARGH crosspost :p.

#15 is a bit better. ;)

So sad. Vancode is a tiny bit of our history.

Do you understand the problem with it Robert?
Its part of Drupal history but not part of everyones drupal history (if that makes sense).

Re-uploading so it's clearer which patch to review.

*scooby confused noise*

Taxonomy term URL aliases (PathTaxonomyTermTestCase) [Path]
The test did not complete due to a fatal error. Completion check path.test 190 PathTaxonomyTermTestCase->testTermAlias()

I just ran this test locally with latest pull, patch applied, and it doesn't fail.

#22: vancode-alphadecimal-1019928-15.patch queued for re-testing.

Tag as #drupalhistory, please

Looks like the patch passed. Do you need user testing?

I also think this is without much merit. Must we exchange our history for dry toast? Do you really get pleasure from the term alphadecimal? Some things are better left as is.

I applied the patch, created a node, and then proceeded to test threaded comments down 3 levels and everything is checking out.

Is there anything specific that needs testing beyond that? If not, it works for me. If there are any other considerations, let me know and I can test for them.

Moshe I'm not sure...

Some developers try to google vancode and that is the problem.
We need committer feedback on this...

Using google's keyword tool, this word is only searched 320 times a month.

Incidentally, this issue is result number 2. http://www.google.com/search?q=vancode
So if anyone googled it, there would be a pretty good chance they'd find out that it was removed.

Using google's keyword tool, this word is only searched 320 times a month.

Incidentally, this issue is result number 2. http://www.google.com/search?q=vancode
So if anyone googled it, there would be a pretty good chance they'd find out that it was removed.

So every single month, 320 Drupal developers are confused by it?

In any case, the current function names are not self-explanatory nor documented. They also violate our naming conventions in several ways: http://drupal.org/coding-standards#naming

I totally support this patch. I spent a couple of hours in comment.module recently and if I hadn't known of this issue I would have been number 321 this month. The point is not that alphadecimal is self-documenting. Vancode sounds like something specific that you should know about if you're using the function. Like when I started programming I didn't know what an integer was, so I looked that up and everything was fine. Looking up vancode gets you nowhere, but looking up alphadecimal works fine. That's the point of this patch.

It's been a while, but my first encounter with "vancode" was pretty confusing too. I eventually found a comment on g.d.o explaining its origins, but only after crawling around in the dark for an hour or so.

This piece of history does not have significace for me personally, but if you really want to preserve it then we ought to at least explain the Vancouver / Unconed "legend" in the documentation.

I'm comfortable committing this patch (and to throw away our vancode history).

I'd say we get rid of it. The small lost in history will be outweighed by the gain in understanding and clarity moving forward.

Since Dries chimed in already I'll let him do the honours on this one.

I'm hopeful we can work on http://drupal.org/project/entity_tree and comment module won't need to maintain its own tree API any more, so this code could end up getting retired during Drupal 8 anyway.

Committed to 8.x.

I went to reference the change notification for this issue and discovered there wasn't one. Oops! The change notification should be pretty straightforward--good novice task.

I'm working on a Change Notification at http://drupal.org/node/1439500

I reviewed the change notification with Transition in IRC. Looks great!