Re: [PATCHv5 2/6] Gitweb: add support for minifying gitweb.css

[Jakub Narebski <jnareb@gmail.com>]

On Wed, 14 April 2010, Mark Rada wrote:
> On 10-04-14 1:22 PM, Jakub Narebski wrote:

> > For git-instaweb I think that best solution would be to introduce new
> > variables holding _source_ of gitweb JavaScript code and CSS, e.g.
> > 
> >             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \
> > 
> > in place of
> > 
> >             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
> > 
> > ...although GITWEB_CSS might mean something different for Makefile
> > and git-instaweb than for gitweb/Makefile and gitweb itself.
> 
> Did you get those lines mixed up? I might be not understanding something
> here.

Ah, I'm sorry, I mixed up those two lines.  They should be in reverse
direction:


             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_SOURCE)' \
 
  in place of
 
             -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS)' \


Actually I'd like to rename @@GITWEB_CSS@@ placeholder etc. in 
git-instaweb.sh, as  @@GITWEB_CSS@@ in git-instaweb.sh means something
quite different from ++GITWEB_CSS++ in gitweb/gitweb.perl...

> 
> I was actually planning something along the lines of 
> 
>              -e '/@@GITWEB_CSS@@/r $(GITWEB_CSS_NAME)' \
>              -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS_NAME)|' \
> 
> where I introduce the GITWEB_CSS_NAME variable, to be consistent with the
> token in instaweb. This way we don't touch GITWEB_JS in the top level
> makefile.

Why not:

            -e '/@@GITWEB_CSS_SOURCE@@/r $(GITWEB_CSS_SOURCE)' \
            -e '/@@GITWEB_CSS_SOURCE@@/d' \
            ...
            -e 's|@@GITWEB_CSS_NAME@@|$(GITWEB_CSS)|' \

(assuming that $(GITWEB_CSS) does not include '|' in it, I guess...
but see below).

> 
> Also, I should update dependancies for instaweb, since those were
> forgotten last time around. Just creating a short list of what the fix will
> need for when I get home tonight.

Something like

   git-instaweb: git-instaweb.sh gitweb/gitweb.cgi $(GITWEB_CSS_SOURCE) $(GITWEB_JS_SOURCE)


P.S. I have noticed additional complication: git-instaweb really needs
gitweb compiled with *specific* values of GITWEB_CSS and GITWEB_JS,
so that they point to git-instaweb's installed files.

-- 
Jakub Narebski
Poland