Opened 6 hours ago
Last modified 4 hours ago
#36587 new Cleanup/optimization
Incorrect sentence in documentation about upload handlers — at Version 2
Reported by: | Baptiste Mispelon | Owned by: | |
---|---|---|---|
Component: | Documentation | Version: | 5.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
The documentation about changing upload handlers on the fly states:
For instance, suppose you’ve written a ProgressBarUploadHandler that provides feedback on upload progress to some sort of AJAX widget. You’d add this handler to your upload handlers like this:
request.upload_handlers.insert(0, ProgressBarUploadHandler(request))
You’d probably want to uselist.insert()
in this case (instead ofappend()
) because a progress bar handler would need to run before any other handlers. Remember, the upload handlers are processed in order.
The part that says "You'd probably want to use list.insert [...] (instead of append)" is incorrect, since the example is already using list.insert
. The sentence should just say something like:
We use
list.insert
to make sure the progress bar handler is run before any other handlers, since the upload handlers are processed in order.
Change History (2)
comment:1 by , 5 hours ago
comment:2 by , 4 hours ago
Description: | modified (diff) |
---|---|
Triage Stage: | Unreviewed → Accepted |
Thank you Baptiste, I see the oddity in the phrasing, though I also understand Tim's point that the sentence is trying to say "do not use append here even if you are tempted to do so".
Accepting since this could be better phrased, my proposal (as a starting point):
Using
list.insert()
, as shown above, ensures that the progress bar handler is placed at the beginning of the list. Since upload handlers are executed in order, this placement guarantees that the progress bar handler runs before the default handlers, allowing it to track progress across the entire upload.
I wouldn't say it's incorrect. I think the meaning is that "[In this hypothetical example,] you'd probably want to [and this example does] use..." but if you found it confusing, perhaps it's fine to change. I'd try to avoid "We" since that's not really the documentation's style.