Re: [PATCHv3 2/5] gitweb: Describe CSSMIN and JSMIN in gitweb/INSTALL

On Mon, 6 June 2011, Jonathan Nieder wrote:

Thank you very much for your review and comments.

Jakub Narebski wrote:

quoted

The build-time configuration variables JSMIN and CSSMIN were mentioned
only in Makefile; add their description to gitweb/INSTALL.

This required moving description of GITWEB_JS up, near GITWEB_CSS and
just introduced CSMIN and JSMIN.

Why does this require that?  Ah, to make the analogy with GITWEB_CSS
clear and because the JSMIN description refers to it.

Yes, moving description of GITWEB_JS was required because I wanted to
cover both of CSSMIN and JSMIN with one entry, which entry of course
refers to both GITWEB_CSS and GITWEB_JS.

quoted

--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL

@@ -147,6 +147,19 @@ You can specify the following configuration variables when building GIT:

[...]

quoted

+ * CSSMIN, JSMIN
+   Invocation of a CSS minifier or a JavaScript minifier, respectively,
+   working as a filter (source on standard input, minified result on
+   standard output).  If set, it is used to generate a minified version of
+   'static/gitweb.css' or 'static/gitweb.js', respectively.  *Note* that
+   minified files would have *.min.css and *.min.js extension, which is
+   important if you also set GITWEB_CSS and/or GITWEB_JS.  [No default]

When I first read this, I thought it meant these command lines were
going to be cooked into the gitweb script and invoked at run time.
Maybe (sorry for the rough text):

* CSSMIN, JSMIN
  Command for a CSS minifier or a Javascript minifier, working as a
  filter [...]
  These are used if defined to generate smaller, non human-readable
  versions of 'gitweb/gitweb.css' and 'static/gitweb.js' at
  'static/gitweb.min.css' and 'static/gitweb.min.js'.  Only the minified
  versions are installed, which is important if you also set GITWEB_CSS
  or GITWEB_JS.  [No default]

Aside from that, looks good.  Thanks.

Thanks.  I'll incorporate those comments in next round... or as a separate
improvement (it is not that bad, I think, to not allow it to be fixed
"in tree").

Anyway the description could use some improvements.  We need to cover the
following issues:

1. CSSMIN and JSMIN are invoked during building gitweb.

2. CSSMIN and JSMIN are interpreted as shell commands, so
   * if you refer to script by full path, you need to quote spaces
     yourself, e.g. 

     JSMIN="'c:/Program Files/JSMin/jsmin.exe'"

   * you can provide options, so you can e.g. use

     JSMIN="perl -MJavaScript::Minifier=minify -we 'minify(input => *STDIN, output => *STDOUT);'"

3. CSSMIN and JSMIN must accept source on standard input, and print
   minified version to standard output, i.e. they are invoked as

     $(CSSMIN) <$< >$@

4. Minified files will be named static/gitweb.min.css and
   static/gitweb.min.js, respectively

5. If you do not set GITWEB_CSS and GITWEB_JS, but use CSS and/or JSMIN,
   they would be set to appropriate values automatically

6. The "install" target in gitweb Makefile, and "install-gitweb" target
   in main Makefile, would install minified versions (if any).

7. If you set both CSSMIN and GITWEB_CSS, you have to adjust GITWEB_CSS
   by yourself.  Same for JSMIN and GITWEB_JS.

Help with wording those would be much appreciated, in more compact form.
Thanks in advance.

-- 
Jakub Narebski
Poland (not native English speaker)