Code

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#4355 closed (fixed)

Confusing example in documentation for url template tag

Reported by: Collin Grady <cgrady@…> Owned by: jacob
Component: Documentation Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The documentation for the url template tag shows it linking to a view as 'app_name.client' which would require a very abnormal project setup to be valid - because of this, it can be confusing to newer django users - I was helping someone on IRC that thought the tag did some translation from app.viewname to project.app.views.viewname :)

Attachments (2)

url-doc.patch (1.0 KB) - added by Collin Grady <cgrady@…> 7 years ago.
url-doc.2.patch (1017 bytes) - added by Marc Fargas <telenieko@…> 7 years ago.
stripped the project_name

Download all attachments as: .zip

Change History (12)

Changed 7 years ago by Collin Grady <cgrady@…>

comment:1 Changed 7 years ago by Marc Fargas <telenieko@…>

  • Triage Stage changed from Unreviewed to Ready for checkin

Yes, it's better to stay consistent across the documentation!

comment:2 follow-up: Changed 7 years ago by mtredinnick

  • Patch needs improvement set
  • Triage Stage changed from Ready for checkin to Accepted

Leave the project name out of the change, though, that makes things less portable (because if you move the app to somewhere else on your Python path, you have to change every URLConf entry that refers to it). Including "viewe" makes sense, though.

comment:3 follow-up: Changed 7 years ago by Collin Grady <cgrady@…>

project_name is used elsewhere though, including in that very same example (the include line) - should that be changed also?

comment:4 in reply to: ↑ 2 Changed 7 years ago by Marc Fargas <telenieko@…>

Replying to mtredinnick:

Leave the project name out of the change, though, that makes things less portable (because if you move the app to somewhere else on your Python path, you have to change every URLConf entry that refers to it). Including "viewe" makes sense, though.

I agree that including the project name makes things less portable but starting at the tutorial 01 http://www.djangoproject.com/documentation/tutorial01/#playing-with-the-api we are telling newcomers to include it (mysite in this case) so we should a) recommend it b) don't do it. But being consistent as much as possible on all the docs/

I'd rather go for the "do not recommend including the project name on imports" but on the whole docs ;)

Anyway, updated patch follows.

Changed 7 years ago by Marc Fargas <telenieko@…>

stripped the project_name

comment:5 in reply to: ↑ 3 ; follow-up: Changed 7 years ago by Marc Fargas <telenieko@…>

Replying to Collin Grady <cgrady@the-magi.us>:

project_name is used elsewhere though, including in that very same example (the include line) - should that be changed also?

You're right, project_name is used in the middle of the section the patch correct which makes it sound strange. Let's let triagers decide whether to apply the first patch or making a new one changing all project_name references on templates.txt

(which means: do not apply the second patch as it makes the section sound strange''')

comment:6 Changed 7 years ago by SmileyChris

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

Consistency is good. I say apply the first patch and open a new ticket to remove the project_name references throughout the templates doc file.

comment:7 in reply to: ↑ 5 Changed 7 years ago by mtredinnick

Replying to Marc Fargas <telenieko@telenieko.com>:

... decide whether to apply the first patch or making a new one changing all project_name references on templates.txt

The only other reference to project_name in that whole file is in a completely different context: it's two lines later showing how the project-wide !URLConf file might include the app-specific !URLConf. Not a bad place to use the project name.

There's no inconsistency at all here (tutorials are not the same as reference document; don't conflate the two). Using project name sometimes and not others is not a disaster. Certainly not in this case.

comment:8 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [5353]) Fixed #4355 -- Clarified a confusing example in the use of the url template
tag. Based on a patch from Collin Grady.

comment:9 Changed 7 years ago by Collin Grady <cgrady@…>

But won't only one of the two methods work depending on your pythonpath setup? (unless you're in a file in the project root)

Seems like an important bit of inconsistency :)

comment:10 Changed 7 years ago by mtredinnick

The first line is in the app dir, the second line is in the project's URL conf -- that is clear in the example -- so both have their own directory in their effective Python import path. The docs say "if is is included like this", setting up the precondition. It doesn't say you have to import it like that. We are not spoon-feeding here, intentionally; that drags down the quality of the docs for the other 90% of readers. If somebody need to stop and think for a minute to get the example straight, no harm is done.

There really are far more important problems to fix than going around and around on this. Let's move on, please.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.