Download & Extend
Issues

Remove underscore-to-dash conversion in path arguments for content types

Posted by matt2000 on October 29, 2009 at 12:30am
Project:Drupal core
Version:8.x-dev
Component:node.module
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)
Issue tags:API change, API clean-up, Entity system, Novice

Issue Summary

Everyone keeps saying, "I always wondered about that, but figured some smart person decided it for a reason..."

Change records for this issue

Login or register to post comments

Comments

#1

Posted by matt2000 on October 29, 2009 at 12:32am

This was born from: http://drupal.org/node/617532#comment-2203464

#2

Posted by sun on October 29, 2009 at 12:56am
Title:Remove underscore-to-dash conversion in URL references to content types» Remove underscore-to-dash conversion in path arguments for content types
Version:8.x-dev» 7.x-dev
Category:feature request» bug report
Priority:minor» normal

Ultimately, I'd like to see this fixed as part of #617532: field_ui menu paths doesn't translate underscores in content type names, since this is a fake limitation (and therefore a bug) that never made any sense.

#3

Posted by yched on October 29, 2009 at 1:24am

Wouldn't it break links on existing Views using a 'node:type' argument after a D6 -> D7 upgrade ?

#4

Posted by sun on October 29, 2009 at 3:11pm
Version:7.x-dev» 8.x-dev

webchick overruled the timeline, but provided some background info:

this underscore-to-hyphen thing was asked for by Dries back when the node type part of CCK first got into core in D5. I don't fancy breaking everyone's URLs at this point, so agree with moving this discussion to D8.

#5

Posted by sun on November 7, 2009 at 9:14pm
Version:8.x-dev» 7.x-dev

However, this totally awkward conversion of internal ids can cause a range of unexpected bugs. Identifiers should be unique and not munged in any way.

So let's fix this.

#6

Posted by yched on November 15, 2009 at 3:28pm

Marked #605236: Content type - bundle realname not using machine readable name as duplicate.

I'm not sure I see which clashes you expect. node type names only allow 'a-z_', so converting '_'s to '-'s should not cause any conflict.

#7

Posted by sun.core on January 8, 2010 at 3:14am
Version:7.x-dev» 8.x-dev

#8

Posted by Damien Tournoud on October 1, 2010 at 8:34am
Category:bug report» feature request

Only feature requests and tasks can be assigned to D8 for now, because the Drupal 8 tree is not opened yet.

Fortunately, this is a feature request :)

#9

Posted by klausi on October 25, 2010 at 1:24pm

Subscribing, why did we introduce this conversion in the first place? Is a '-' in a URL prettier than a '_'?

#10

Posted by sun on May 6, 2012 at 5:47pm
Component:node system» node.module
Category:feature request» task

Any takers? This should be relatively simple (just grep entire core for the string replacement and remove them all), so tagging as Novice.

#11

Posted by sun on May 6, 2012 at 6:44pm
Status:active» needs review

Kick-starting this, 'cos I can't stand this bogus beautification anymore.

AttachmentSizeStatusTest resultOperations
drupal8.node-type-url.11.patch28.27 KBIdleFAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/comment/comment.module.View details

#12

Posted by System Message on May 6, 2012 at 6:46pm
Status:needs review» needs work

The last submitted patch, drupal8.node-type-url.11.patch, failed testing.

#13

Posted by sun on May 6, 2012 at 6:57pm
Status:needs work» needs review
Issue tags:+API change, +API clean-up

Created follow-up issue for #1564832: Remove _node_extract_type() and node_type_get_name() (postponed on this one)

AttachmentSizeStatusTest resultOperations
drupal8.node-type-url.13.patch28.26 KBIdleFAILED: [[SimpleTest]]: [MySQL] 31,159 pass(es), 5,210 fail(s), and 13,158 exception(s).View details

#14

Posted by System Message on May 6, 2012 at 7:20pm
Status:needs review» needs work

The last submitted patch, drupal8.node-type-url.13.patch, failed testing.

#15

Posted by sun on May 6, 2012 at 8:37pm
Status:needs work» needs review

Fixed the exceptions.

AttachmentSizeStatusTest resultOperations
drupal8.node-type-url.15.patch30.21 KBIdleFAILED: [[SimpleTest]]: [MySQL] 36,245 pass(es), 195 fail(s), and 79 exception(s).View details

#16

Posted by System Message on May 6, 2012 at 9:01pm
Status:needs review» needs work

The last submitted patch, drupal8.node-type-url.15.patch, failed testing.

#17

Posted by matt2000 on May 6, 2012 at 10:44pm
Status:needs work» needs review

Here's a new patch with fixes for the broken tests. (That is, the new code has not changed; instead this patch only includes updates to old tests that try to get a 'test-bundle' url instead of the new 'test-bundle'.)

AttachmentSizeStatusTest resultOperations
drupal8.node-type-url.17.patch37.81 KBIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.node-type-url.17.patch. Unable to apply patch. See the log in the details link for more information.View details

#18

Posted by System Message on May 6, 2012 at 10:44pm
Status:needs review» needs work

The last submitted patch, drupal8.node-type-url.17.patch, failed testing.

#19

Posted by matt2000 on May 6, 2012 at 10:49pm
Status:needs work» needs review

I guess my patch generation method needs updating... Let's try this one.

AttachmentSizeStatusTest resultOperations
drupal8.node-type-url.19.patch37.96 KBIdlePASSED: [[SimpleTest]]: [MySQL] 36,495 pass(es).View details

#20

Posted by sun on May 7, 2012 at 12:09am

woot! Thanks a lot @matt2000!

@chx requested in IRC to dig out why this conversion exists in the first place. Answer: Purely cosmetic.

Introduced in the initial commit for custom node types for D5: #62340-91: Pave the way for CCK

And even back then, a lot of discussion followed on that "beautification" detail already (search the issue for "dash"). :(

#21

Posted by chx on May 7, 2012 at 12:16am
Status:needs review» reviewed & tested by the community

OK then.

#22

Posted by catch on May 8, 2012 at 5:12am
Title:Remove underscore-to-dash conversion in path arguments for content types» Change notification for: Remove underscore-to-dash conversion in path arguments for content types
Priority:normal» critical
Status:reviewed & tested by the community» active

Oooh I'd not seen this issue. Committed/pushed to 8.x, will need a change notification.

19 files changed, 97 insertions(+), 131 deletions(-)

#23

Posted by tim.plunkett on May 12, 2012 at 7:54pm
Status:active» needs review

Opened two change notices:
http://drupal.org/node/1574668
http://drupal.org/node/1574670

#24

Posted by sun on May 12, 2012 at 8:26pm
Title:Change notification for: Remove underscore-to-dash conversion in path arguments for content types» Remove underscore-to-dash conversion in path arguments for content types
Priority:critical» normal
Status:needs review» fixed

Thanks! I've revised both a bit.

#25

Posted by System Message on May 26, 2012 at 8:30pm
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

nobody click here