Re: [PATCH] git-instaweb: Add option to reuse previous config file

From: Petr Baudis <pasky@suse.cz>
Date: Jun 2, 2010, 09:44 UTC
Message-ID: <20100602094408.GD27342@machine.or.cz>
In-Reply-To: <201006012331.37871.jnareb@gmail.com>

On Tue, Jun 01, 2010 at 07:14:05PM +0530, Pavan Kumar Sunkara wrote:

Show 9 quoted lines

> @@ -551,7 +557,7 @@ our \$projects_list = \$projectroot;
>  EOF
>  }
>  
> -gitweb_conf
> +test "$no_reuse" = true && gitweb_conf
>  
>  resolve_full_httpd
>  mkdir -p "$fqgitdir/gitweb/$httpd_only"

It is a better style to write:

	test "$no_reuse" = false || gitweb_conf

The reason is that the script can then be easily run with set -e. But I know that the rest of git-instaweb doesn't really follow this convention, so this is just a nitpick. (Moreover, why do you have option --reuse-config but variable no_reuse with an oppposite meaning?)

On Tue, Jun 01, 2010 at 11:31:37PM +0200, Jakub Narebski wrote:

Show 7 quoted lines

> If git-instaweb is invoked *without* --reuse-config, the gitweb_config.perl
> would be regenerated whether it exists or not, overwriting your changes.
> 
> If git-instaweb is invoked *with* --reuse-config, the gitweb_config.perl
> would be generated if it does not exist (so if you delete gitweb_config.perl
> and then run 'git instaweb --reuse-config' it would not fail), and reused
> if it does exist.

That sounds like a good idea, but I think at that point --reuse-config gets to be a way too misleading name. --keep-config might be tiny bit better, but not a lot.

>   test "$no_reuse" = true || test ! -e "$GITWEB_CONFIG" && gitweb_conf
> 
> or something like that (the test if $GITWEB_CONFIG file exists might be
> moved to the fragment of code that sets no_reuse to true instead).

BTW, I think at the point you have to chain more than two pieces at once in the boolean sequence, it gets clearer to write an if. Just my personal opinion, though. (Mainly because I can never remember the priorities. ;-)

-- 
				Petr "Pasky" Baudis
The true meaning of life is to plant a tree under whose shade
you will never sit.