Templatise describekeywords.cgi

RESOLVED FIXED in Bugzilla 2.16

Status

()

Bugzilla
Bugzilla-General
P1
blocker
RESOLVED FIXED
17 years ago
6 years ago

People

(Reporter: gerv, Assigned: gerv)

Tracking

2.15
Bugzilla 2.16

Details

Attachments

(1 attachment, 3 obsolete attachments)

You are my template, my only template, you keep my happy when skys are grey...

Gerv
Created attachment 63125 [details] [diff] [review]
Patch v.1

This one was pretty easy.

Gerv
Priority: -- → P1
Target Milestone: --- → Bugzilla 2.16
Keywords: patch, review

Comment 2

17 years ago
I've taken a look at this, and it's all plausible. r=afranke if that helps.

However, I've tried to apply it, and my patch program rejected the first hunk;
no idea why. 

Another thing: you are using an "html" filter for the keyword in the url;
shouldn't this be a "uri" filter or something?

And I think I've seen a bug with a patch to CGI.pl that introduced a global
template object; maybe the patch can be simplified a bit if that one lands
before this one?
Created attachment 67597 [details] [diff] [review]
Patch v.2

Updated version with =afranke.

Gerv
Attachment #63125 - Attachment is obsolete: true

Comment 4

17 years ago
Created attachment 67884 [details] [diff] [review]
cvs diff -u describekeywords.cgi , using $::template and $::vars (v.3)

This is like it's done in enter_bug.cgi. 

Since I was unable to produce a combined patch with describe_keywords.tmpl,
I will upload it separately, even though it's the same as in Patch v.2.

Updated

17 years ago
Attachment #67884 - Flags: review-

Comment 5

17 years ago
Comment on attachment 67884 [details] [diff] [review]
cvs diff -u describekeywords.cgi , using $::template and $::vars (v.3)

Grrr, I think I'll leave it to Gerv to attach patches. This patch changes the
path to perl, so marking needs-work. :-(

Comment 6

17 years ago
Comment on attachment 67884 [details] [diff] [review]
cvs diff -u describekeywords.cgi , using $::template and $::vars (v.3)

I have some comments here, but it looks in general ready to go.
If these are fixed, r=kiko.

>--- describekeywords.cgi	2002/01/20 01:44:37	1.6
>+++ describekeywords.cgi	2002/02/05 10:56:22
>@@ -1,4 +1,4 @@
>-#!/usr/bonsaitools/bin/perl -wT
>+#!/opt/perl5.0.0.4/bin/perl -wT

Eeek. :-)

>+   
>+    # This 'onebug' stuff is silly hackery for old versions of
>+    # MySQL that seem to return a count() of 1 even if there are
>+    # no matching. So, we ask for an actual bug number. If it

s/matching/matches/

Can we drop this now that we require a modern version of MySQL as
per checksetup.pl?

>+print "Content-type: text/html\n\n";
>+$template->process("info/describe_keywords.tmpl", $vars)
>+  || DisplayError("Template process failed: " . $template->error())
>+  && exit;


I think we decided on .html.tmpl, right? :-)
Attachment #67884 - Flags: review-
>Can we drop this now that we require a modern version of MySQL as
>per checksetup.pl?

Yes, it looks like this bug was fixed in 3.21.22, and we require 3.22.5.


>I think we decided on .html.tmpl, right? :-)

True for templates with formats, but otherwise we didn't decide.

Comment 8

17 years ago
Strange, I'm pretty sure I did mark my patch needs-work, but in the activity log
it shows up as kiko. Known bug?
We should go for .html.tmpl in all cases, I think, because then templates can
acquire formats with minimal fuss. Myk - is that reasonable?

(/me sees us renaming all the templates before release...)

Gerv
Another possibility: If you are looking for an html template, and you can't find
a .html.tmpl, then look for a .tmpl also. But this may be a bit slower. 

Having everything as .html.tmpl is probably the cleanest solution.
Created attachment 68598 [details] [diff] [review]
Patch v.3

Try this. Uses $::templates and $::vars, the ancient cruft is removed, and the
template is now called describe-keywords.html.tmpl , in line with the latest
thinking on file naming.

Gerv
Attachment #67597 - Attachment is obsolete: true
Attachment #67884 - Attachment is obsolete: true
>We should go for .html.tmpl in all cases, I think, because then templates can
>acquire formats with minimal fuss. Myk - is that reasonable?

Sounds good to me.

>(/me sees us renaming all the templates before release...)

Yes, we have to do this anyway for the .atml templates, so we should bundle all
the changes together and do them all at once (and wait until the end in case
some other change comes up).

Comment 13

17 years ago
Comment on attachment 68598 [details] [diff] [review]
Patch v.3

2r=kiko and marking r=afranke. Looks good for check-in.
Attachment #68598 - Flags: review+
Checking in describekeywords.cgi;
/cvsroot/mozilla/webtools/bugzilla/describekeywords.cgi,v  <--  describekeywords.cgi
new revision: 1.7; previous revision: 1.6
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/default/info/describe-keywords.html.tmpl,v
done
Checking in template/default/info/describe-keywords.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/default/info/describe-keywords.html.tmpl,v
 <--  describe-keywords.html.tmpl
initial revision: 1.1
done

Gerv
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.