Linkify script to use quoteUrls on texts provided by user

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Bugzilla-General
P2
enhancement
RESOLVED FIXED
16 years ago
5 years ago

People

(Reporter: Stefan Seifert, Assigned: Stefan Seifert)

Tracking

unspecified
Bugzilla 2.18

Details

(URL)

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

16 years ago
using kiko's description:
a page called bugify.cgi that offers a textarea, that works like this: enter
text with numbers interspersed and the numbers become links with title="summary"
added. this is for helping asa produce daily summaries and the weekly updates.
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Priority: -- → P2
(Assignee)

Comment 1

16 years ago
Created attachment 56994 [details]
first version, would appreciate review

Comment 2

16 years ago
Not a review, but I liked the idea, so some minor comments...

1) 'Name "main::usergroupset" used only once: possible typo at...' server log
warning

2) Should it (could it) run in taint mode?

3) (typo) apropriate -> appropriate

4) Captilize 'this', in 'this page lets...' maybe?

What does query/buglist fail to provide for this?

Comment 4

16 years ago
MattyT, this is a generic textarea that swaps numbers for links with TITLEs set
-- it's of use to people that prepare textual summaries that include bug numbers
such as Asa's nightly build reports and the mozilla status report -- just
paste in a lot of text with numbers (you could paste in the full status report
if you wanted to) and get back the text with <A TITLE="" HREF="">666</A> in place
of the boring little numbers.

I'm sure this is of use to a lot of people (justdave said it "was cool")
OK, I'd assumed it was a link on the page as opposed to the link text.  Perhaps
this should be placed in a 'Bugzilla Utilities' section off the main page.  I'm
not sure what else we'd put there though, any suggestions?
Keywords: patch, review
Target Milestone: --- → Bugzilla 2.16

Comment 6

16 years ago
This is very similar to, if not a dupe of bug#101864, isn't it?

Comment 7

16 years ago
I've already written a full-blown script that does just this by querying
bugzilla with LWP.  It's what Asa is currently using to pre-process the build
bar comments.   You can access it at http://www.xsta.cc/bugreport.html with
source at http://www.xsta.cc/bugreport.cgi, feel free to add to the Bugzilla
utilities page.  Various keywords like bugzilla, #mozillazine,  etc. are
recognized and appropriate links inserted.  It will only handle 20 bug links at
a time as there seems to be a limit on bugzilla to prevent DoS attacks.

In bug #101864 (which is a dupe) it was suggested that this didn't belong in
Bugzilla, which is I implemented this separately.
http://www.xsta.cc/bugreport.cgi is 403 forbidden.

This definitely belongs in Bugzilla because it has wide utility - running the
weekly status reports through it would improve them immensely. 

Review comments:

It should be called linkify.cgi. :-)
It should not linkify bug numbers which do not exist. But maybe that's another bug. 
It should contain a full MPL Exhibit A, referencing you as the original author
of the code (assuming you didn't copy any from anywhere else.)

Gerv

Comment 9

16 years ago
> http://www.xsta.cc/bugreport.cgi is 403 forbidden.
make that http://www.xsta.cc/bugreport.pl

I will have to rewrite it to be a part of bugzilla, the current method doesn't
touch the database server at all.  I'll try and get that done over the weekend,
if someone wants to adapt my code to work under bugzilla before then, feel
free.I'll address your comments during the rewrite.
There is code in Bugzilla to do the quoting - you just need to call the
appropriate function. Please don't re-invent the wheel there. 

Any new CGIs checked into Bugzilla will need to be templatised. If you need help
with that, ask us. Check attachment.cgi in the current Bugzilla for examples of
how to do it.

Use an action= parameter to determine what the script should do; show the text
box by default.

Hope that helps :-)

Gerv
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
(Assignee)

Comment 12

16 years ago
Oh...just one week away and something like this happens.

Just a question: who should do this now? Would be no problem for me if Hampton
does this but I'm currently working on a new version which doesn't use quoteUrls
but GetBugLink cause the former likes to destroy the status update page if used
on the complete page.

Using templates is no problem, it's far better than the current prints ;)

Comment 13

16 years ago
Go ahead and take this.  I've got a lot on my hands right now and would need to
get more familiar with Bugzilla before I could do this.
(Assignee)

Comment 14

16 years ago
Okay new version ready: converted it to use the Template Toolkit. It now uses
action=raw instead of raw=1 and most important uses GetBugLink instead of
quoteUrls so you can process a complete weekly status update page :)

And it's called linkify as of Gerv's comment ;)

Bugs which don't exist are linked. To avoid this requires patching GetBugLink()
Summary: script to use quoteUrls() on texts provided by user → Linkify script to use GetBugLink on texts provided by user
(Assignee)

Comment 15

16 years ago
Created attachment 60158 [details]
New version, adresses comments

cannot make attachment 56994 [details] obsolete, don't have sufficient rights.
(Assignee)

Comment 16

16 years ago
Created attachment 60159 [details]
Template for linkify.cgi script

place it in template/default/linkify/

Updated

16 years ago
Attachment #56994 - Attachment is obsolete: true

Comment 17

16 years ago
GetBugLink() was patched in bug 86300 to not link a bug if it doesn't exist
(this patch hasn't landed on b.m.o yet, so bug 1 is still a link until it does
land :)
Stefan: you now have the necessary permissions; but I don't have time to review
your patch :-(

Gerv
*** Bug 101864 has been marked as a duplicate of this bug. ***
Comment on attachment 60158 [details]
New version, adresses comments

The template calling system has changed since this patch was created, and we
also have moved the template directory into language-specific directories.  The
template code is glocal now so I think you just need to add stuff to $vars and
call $template->process now (search path should be set for you already)
Attachment #60158 - Flags: review-
Comment on attachment 60159 [details]
Template for linkify.cgi script

Template looks good, but is Myk really the contributor?  This file was uploaded
by Stefan by the look of it...
The template is badly formatted.

The CGI:
- duplicates loads of stuff from e.g. globals.pl
- is dual-licensed, when the standard Bugzilla license is the MPL
- Uses GetBugLink, when quoteUrls would have been more appropriate

It's far simpler than this makes out. You could almost implement it using
page.cgi, if you made quoteUrls available to everyone (which is reasonable; it's
used in 3 CGIs already.)

Gerv

Created attachment 94782 [details] [diff] [review]
Patch v.1

This is a "page.cgi"-based version. It's all templates apart from two tweaks,
which are reasonable generic things to do.

Gerv
... and here you see the problem with adding this stuff into checksetup - new
installations won't get this.

Apart form taht, the comment on quoteUrls is wrong, and if you add it in
globally, you should pull it out of long_list and bug_form. (Of course, it
should be a FILTER, but thats another bug)
> ... and here you see the problem with adding this stuff into checksetup - new
> installations won't get this.

OK, well it's not too late to change the mapping mechanism. What do you suggest?

Another option is:
template/en/default/pages/<id>.tmpl.

and the ids acquire a content type, e.g.:
page.cgi?id=linkify.html

That has a certain neatness to it.

Gerv
You could do that, or just not allow formats for page stuff. The main advantage
for formats is that you can share the same collected data, but there is no
collected data for pages....

How were you planning to get teh content type for pages anyway? Its currently
hardcoded to text/html.
There's a rather useful mapping in localconfig.

I'll open another bug for this.

Gerv

Updated

16 years ago
Depends on: 162151
Created attachment 99714 [details] [diff] [review]
Patch v.2

Now this bug's dependencies are fixed, we can have this. 

It's basically a couple of files for page.cgi, plus a few tweaks, and some
fixes to page.cgi. For such a small file, I seem to be getting it wrong rather
a lot. Anyway.

Gerv
Attachment #60158 - Attachment is obsolete: true
Attachment #60159 - Attachment is obsolete: true
Attachment #94782 - Attachment is obsolete: true

Comment 29

15 years ago
Comment on attachment 99714 [details] [diff] [review]
Patch v.2

>-if (defined $::FORM{'id'}) {
>+if ($::FORM{'id'}) {
>     $::FORM{'id'} =~ s/[^\w\-\.]//g;
>-    $::FORM{'id'} =~ /(.*)(\.(.*))?/;
>+    $::FORM{'id'} =~ /(.*)\.(.*)/;

This section deserves a comment. I *think* I know what it does
(removes anything non-alphanumeric, - and ., get two blocks
separated by dots) but I'm not sure why. Just an overall comment.

>+[% INCLUDE global/header.html.tmpl title = "Linkify Text" %]
>+
>+<p>
>+  If you enter some text, this form will return it with valid bug numbers, URLs
>+  and so on replaced with appropriate HTML links.
>+</p>

This text should explain that bug numbers should follow the format "bug 54332".
What does "and so on" mean? Maybe:

This form will add HTML links to bugs and URLs.

Hint: to linkify a bug, use the format <CODE>bug <I>bug_id<I></CODE>, as in 
<CODE>bug 3242</CODE> or <CODE>bug gerv_rocks<CODE>.

>+<form action="page.cgi" method="post">
>+  <textarea cols="100" rows="20" name="text"></textarea>
>+  <br>    
>+  <input type="hidden" name="id" value="linked.html">
>+  <input value="Linkify" type="submit">
>+</form>

We could get away with 80 columns and not make it horizontally-scroll on
640px displays. I would *really* like that change.

The patch looks and works fine, r=kiko. No need for 2r=.
Attachment #99714 - Flags: review+
Fixed. I looked carefully; this shouldn't conflict with groups.

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.202; previous revision: 1.201
done
Checking in page.cgi;
/cvsroot/mozilla/webtools/bugzilla/page.cgi,v  <--  page.cgi
new revision: 1.7; previous revision: 1.6
done
Checking in long_list.cgi;
/cvsroot/mozilla/webtools/bugzilla/long_list.cgi,v  <--  long_list.cgi
new revision: 1.30; previous revision: 1.29
done
Checking in bug_form.pl;
/cvsroot/mozilla/webtools/bugzilla/bug_form.pl,v  <--  bug_form.pl
new revision: 1.103; previous revision: 1.102
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.145; previous revision: 1.144
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linked.html.tmpl,v
done
Checking in template/en/default/pages/linked.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linked.html.tmpl,v
 <--  linked.html.tmpl
initial revision: 1.1
done
RCS file:
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linkify.html.tmpl,v
done
Checking in template/en/default/pages/linkify.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/linkify.html.tmpl,v
 <--  linkify.html.tmpl
initial revision: 1.1
done

Gerv
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Summary: Linkify script to use GetBugLink on texts provided by user → Linkify script to use quoteUrls on texts provided by user
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.