Date: Sun, 15 Feb 2004 14:10:26 +0200 From: Ville =?ISO-8859-1?Q?Skytt=E4?= <scop@FreeBSD.org> To: wolf@ti.com Cc: freebsd-cvsweb@FreeBSD.org Subject: Re: Issues/patches for 2.9.2 Message-ID: <1076847025.9498.262.camel@bobcat.mine.nu> In-Reply-To: <402D62C4.3050300@ti.com> References: <402D62C4.3050300@ti.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 2004-02-14 at 01:50, Christopher Wolf wrote: > I installed the Beta cvsweb.cgi a few days ago and found a few issues > I'd like > to suggest improvements to. Below are some context diffs and comments on > each issue. Thanks! Comments below. > *With GNU diffutils version 2.7 and an 80 character wide development > environment(!), 164 characters is not wide enough for side-by-side > diffs. A change to 168 shows all 80 columns with room for the diff > indicators:* Applied. > *When trying to download a tarball/zipfile, the cvs command checks the > files out into one directory, and the zip/tar command tries to read > another: the cvs command checks out the module tree path > (i.e. master/rts/shared/) while the zip/tar command refers to the base > module (i.e. shared/ in this example). This change modifies the cvs > checkout command to use the same name as the tar/zip, although a more > complex fix is probably needed: > * Applied a version that restores the way it was handled in revisions before 1.191 (ie. FreeBSD-CVSweb 2.9.1); was accidentally broken there. It's pretty much similar as the patch you sent, and even a bit simpler (using $basename directly instead of File::Basename). > When the configuration variable $shortLogLen is set to something > longer (such as 500), the file and age columns in the directory view > will wrap, which is hard to read. This change makes those columns > nowrap, so that the log message is what wraps. This could be added to > the userid columns too, but those seldom contain spaces.* The patch you sent was not valid XHTML (we send XHTML 1.0 Transitional nowadays), and since the nowrap attribute is deprecated already in HTML 4, I reimplemented it using CSS and committed a couple of other related enhancements while at it. > When a particular revision is chosen from a revision tree graph, most > of the log links are turned off! I could not see a reason why, and > the developers here wanted to use the tree to find a particular > revision based on tag names, and then see the differences that made > that revision. This change adds a config file variable that allows > turning on those links:* I don't see why they were turned off either, and committed a change that accomplishes what you suggested, ie. just does The Right Thing without adding a config param for it. > When enscript's -x option is set to 'x', the resulting image map code > is not compatible with the newer mozilla browsers, which are fairly > particular about using 'name' vs 'id' for identifying the image map. > This change calls out to use both name and id and the result is > compatible with IE and Mozilla:* Unfortunately changing the cvsgraph option to "-x 4" makes it output HTML 4 map markup which not valid XHTML. I put in a kludge which adds the name attribute to the map element on the fly, and still preserves the "-x x" option to cvsgraph and XHTML compatibility. > The colors used to display diffs are hard to read, so I made the > following change to force changes in diffs to appear with a grey > background, making them much easier to pick out when scrolling large > files:* Applied, modifying it so that I added another enscript highlight file, lang_cvsweb_diff.st which is used to colorize diffs only. The background color change did not look too good in non-diff code colorization. > I also found that enscript's regexps in diffs.st would often, when > displaying side-by-side colors diffs, get confused by lines of code > ending in | (think OR) or > (think #include <xxx>) if it had a trailing > tab. This file is not distributed with CVSweb, so I'd suggest that you send those modifications to the Enscript author(s), see http://people.ssh.com/mtr/genscript/ Thanks again for the patches, I'll release a new beta including these changes today.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1076847025.9498.262.camel>