Re: CVSweb patches

[Ville Skyttä <scop@xxxxxxxxxxx>]

On Sat, 2005-12-03 at 01:52 -0800, Daniel Qarras wrote:

> I've attached a couple of patches against CVSweb 3.0.6, please apply
> them if you find them useful.

Thanks, some initial comments below:

> a) Colored age column to indicate how recently a file has been
> modified. See [1] for a similar feature.

Nifty.  However, I'm inclined to drop the newly introduced
$use_color_age config variable; the intention is that presentation
tweaks would be done using only CSS as much as possible.  Folks who
don't like the coloring can just set the CSS attributes to the same for
all age classes.  Additionally, the markup/CSS implementation could be
done using "multi-class" approach to reduce redundancy, for example

   <td class="age years">

   table.dir * td.age {
     font-style: italic;
     white-space: nowrap;
   }
   table.dir * td.years {
     color: #000;
   }

> b) Change default colors to match ViewCVS and other CVSweb
> implementations. See [1] for a similar color scheme.  The current
> default colors are, in my eyes, not very appealing but YMMV.

No opinion on this yet (not tested), but just a nitpicky note: the
colors in the default CSS are intentionally specified using the three
char #xyz values in order to remind sticking to a set of colors that'll
work in an expected'ish way with non-truecolor displays.

> b) log_accum script used to send CVS commit messages uses revision NONE
> for newly added files which causes an error with current CVSweb.
> Prevent the error with the patch. Could be probably done better but at
> least this avoids the error situation.

Hm.  Even if that particular error could be prevented using this patch,
"NONE" is not a meaningful general revision identifier.  For example:

http://www.freebsd.org/cgi/cvsweb.cgi/projects/cvsweb/README.diff?r1=NONE&r2=1.1
(Just to demonstrate; that instance of cvsweb does not do the early
query string param filtering that 3.0.x does.)

I'm not sure if it's actually possible to sanely retrieve "no revision"
-> "initial revision" file additions as diffs from a CVS/RCS repository
in a generic way.  If there is a way, this patch should be extended so
that "NONE" would be special cased so it could be used as a "meta
revision" of a kind to accomplish this.

On the other hand, it could make sense to "fix" log_accum so that it
wouldn't generate diff links for file additions, but would rather point
to some other view of the newly added file in cvsweb, or omit the links
altogether.

Thoughts welcome, and thanks again for the patches.