Some of discussion is repeated in another subthread
On Sun, Mar 18, 2007, Martin Koegler wrote:
> On Sat, Mar 17, 2007 at 07:04:54PM +0100, Johannes Schindelin wrote:
>> On Sat, 17 Mar 2007, Martin Koegler wrote:
>>
>>> The whole is implemented in JavaScript on the client side (tested with
>>> IE 6 and Mozilla).
>>
>> This is not acceptable for me. There are too many people blindly running
>> JavaScript everywhere, and we should not enforce such bad behaviour.
>>
>> At least, you need a way to do it with plain old HTML (which is easy
>> enough).
>
> If the browser supports no JavaScript, the user can use all current
> features, except the new diff capabilities.
>
> My first thoughts where also about implementing it inside
> gitweb.cgi. This would mean a large change to the code, as all code
> which generate links needs to be adapted.
>
> Additionally, selecting a base object would mean, that you must submit
> a request to the server and probably return something (probable the
> same page, on which you selected the base object).
First, generating gitweb links in gitweb _all_ goes through href()
subroutine, so it would be fairly easy to implement it server side,
i.e. directly in gitweb.perl. One solution would be to set cookies
for parent (base) of diff server side. Another would be to pass
through diff parent (diff base) options (CGI parameters) in non-diff
views. It means that for example shortlog, or history, or summary
view would get 'hp', 'hpb', 'fp' parameters and pass it through
to "diff to selected base" links. But it is a fact that it would
mean additional request to the server to set the base object.
The correct solution I think which would satisfy everyone would
be to add plain HTML, i.e. server-side solution to "diff with
selected base" if this option is turned on. Then JavaScript would
replace it by JavaScript solution (or would add JavaScript solution
if server-side solution is turned off), again subject to gitweb
configuration. JavaScript would use cookies if possible, and changing
location if not (that is for discussion).
> For me, this would be no problem, but for high load servers (eg. gitweb at
> kernel.org), it would incread the system load.
I think use JavaScript if possible, and server-side implementation
if not would be good compromise.
> With JavaScript, this step needs no server interaction. Infact, a
> client could add the diff feature to any gitweb server with
> eg. greasemonkey, by injecting the JavaScript code.
Well, we could always put this in contrib/ as Greasmonkey script ;-)
>> Also I'd like to know: is there any reason you did not send a proper diff,
>> given that you are so interested in diffs?
>
> I'm not interessted in generating patchs via gitweb. I want to examine
> and review differences between various branches/tags/commits/... .
>
> The "pseudo" diff for gitweb.cgi was to illustrate, how to integrate
> gitweb.js. The whole thing is under development, so I have not created
> a clean version.
I'm just wondering why did you not used git for development. You have
to have git installed to test gitweb, and it would be best to have
git.git repository fetched to base your work on newest 'master' version
(or rather 'origin/master' or 'origin' version). So it would be natural
to use git to work on gitweb, and to send git patches.
> As I wrote in my last mail, I used blobdiff to generation tree diffs,
> which results in wrong/missing file names. In the mean time, I've
> create a first version of a treediff function for gitweb.
Which is IMVHO long way from ready.
> --- old/gitweb.js 2007-03-17 15:18:23.284317140 +0100
> +++ gitweb.js 2007-03-17 22:25:18.254190096 +0100
First, if you don't use git to generate diffs, could you _please_
use equal depth paths patches, i.e. either
--- gitweb.js.orig 2007-03-17 15:18:23.284317140 +0100
+++ gitweb.js 2007-03-17 22:25:18.254190096 +0100
or
--- old/gitweb.js 2007-03-17 15:18:23.284317140 +0100
+++ new/gitweb.js 2007-03-17 22:25:18.254190096 +0100
> --- ../mirror/git.git/gitweb/gitweb.perl 2007-03-12 22:06:44.000000000 +0100
> +++ gitweb.cgi 2007-03-17 18:41:50.000000000 +0100
The same as above (or even worse)
> @@ -1835,6 +1837,7 @@
> close $fd;
> }
>
> + print '<script type="text/javascript" src="gitweb.js"></script>';
> print "</body>\n" .
> "</html>";
> }
I'd rather have end of line after closing '</script>', and have scripts
in the head section, not in body.
> @@ -4191,6 +4194,106 @@
> git_commitdiff('plain');
> }
>
> +sub git_treediff {
> + my $format = shift || 'html';
[...]
You base git_treediff on git_commitdiff. On one hand it is right, as
treediff is like commmitdiff, but without commit message, nor link to
commit parents.
On the other hand you should be able to handle 'hb', 'hpb', 'f', 'fp'
parameters like git_blobdiff does. And making git_difftree work between
two different trees from two different commits (e.g. git-gui/ in git.git
mainline and / in git-gui.git mainline) is not that easy...
--
Jakub Narebski
Poland
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html