| Message ID | 20181216232429.GJ75890@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gitweb: correctly store previous rev in javascript-actions mode | expand |
From: Robert Luberda <robert@debian.org> Date: Sun, 16 Mar 2014 22:57:19 +0100 Without this change, the setting $feature{'javascript-actions'}{'default'} = [1]; in gitweb.conf breaks gitweb's blame page: clicking on line numbers displayed in the second column on the page has no effect. For comparison, with javascript-actions disabled, clicking on line numbers loads the previous version of the line. Addresses https://bugs.debian.org/741883. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Robert Luberda <robert@debian.org> --- > Hi Robert, > Years ago, you sent this obviously correct patch to the link mentioned > above, but it got lost in the noise. Sorry about that. Hopefully > late is better than never. Hi, Somehow I missed your e-mail and just have found it today by a chance :( > May we forge your sign-off? See https://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off for more details about what this means. Done, I've added the Signed-off-line above. > Jakub et al, any thoughts? I don't see any unit tests in gitweb/static > that could avoid this regressing --- am I missing some, or if not any hints for someone who would want to add a test framework? Thanks, Robert gitweb/static/js/blame_incremental.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitweb/static/js/blame_incremental.js b/gitweb/static/js/blame_incremental.js index db6eb50584..e100d8206b 100644 --- a/gitweb/static/js/blame_incremental.js +++ b/gitweb/static/js/blame_incremental.js @@ -484,7 +484,7 @@ function processBlameLines(lines) { case 'previous': curCommit.nprevious++; // store only first 'previous' header - if (!'previous' in curCommit) { + if (!('previous' in curCommit)) { var parts = data.split(' ', 2); curCommit.previous = parts[0]; curCommit.file_parent = unquote(parts[1]);
Robert Luberda <robert@debian.org> writes: > From: Robert Luberda <robert@debian.org> > Date: Sun, 16 Mar 2014 22:57:19 +0100 > > Without this change, the setting > > $feature{'javascript-actions'}{'default'} = [1]; > > in gitweb.conf breaks gitweb's blame page: clicking on line numbers > displayed in the second column on the page has no effect. > > For comparison, with javascript-actions disabled, clicking on line > numbers loads the previous version of the line. > > Addresses https://bugs.debian.org/741883. > > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> > Signed-off-by: Robert Luberda <robert@debian.org> For what it is worth it (because I am not active in gitweb development): Acked-by: Jakub Narębski <jnareb@gmail.com> > --- >> Hi Robert, > >> Years ago, you sent this obviously correct patch to the link mentioned >> above, but it got lost in the noise. Sorry about that. Hopefully >> late is better than never. > > Hi, > > Somehow I missed your e-mail and just have found it today by a chance :( > >> May we forge your sign-off? See >> https://www.kernel.org/pub/software/scm/git/docs/SubmittingPatches.html#sign-off >> for more details about what this means. > > Done, I've added the Signed-off-line above. Thanks for following this up. >> Jakub et al, any thoughts? I don't see any unit tests in gitweb/static >> that could avoid this regressing --- am I missing some, or if not any >> hints for someone who would want to add a test framework? We currently have no tests for the JavaScript in gitweb code; I am not sure how one would go to add such tests (and whether it would be possible while gitweb is part of git - if they need externel dependencies like Node.js or Selenium they would need to be able to be disabled or enabled with builld option). > gitweb/static/js/blame_incremental.js | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gitweb/static/js/blame_incremental.js > b/gitweb/static/js/blame_incremental.js > index db6eb50584..e100d8206b 100644 > --- a/gitweb/static/js/blame_incremental.js > +++ b/gitweb/static/js/blame_incremental.js > @@ -484,7 +484,7 @@ function processBlameLines(lines) { > case 'previous': > curCommit.nprevious++; > // store only first 'previous' header > - if (!'previous' in curCommit) { > + if (!('previous' in curCommit)) { > var parts = data.split(' ', 2); > curCommit.previous = parts[0]; > curCommit.file_parent = unquote(parts[1]); Thanks,
diff --git a/gitweb/static/js/blame_incremental.js b/gitweb/static/js/blame_incremental.js index db6eb50584..e100d8206b 100644 --- a/gitweb/static/js/blame_incremental.js +++ b/gitweb/static/js/blame_incremental.js @@ -484,7 +484,7 @@ function processBlameLines(lines) { case 'previous': curCommit.nprevious++; // store only first 'previous' header - if (!'previous' in curCommit) { + if (!('previous' in curCommit)) { var parts = data.split(' ', 2); curCommit.previous = parts[0]; curCommit.file_parent = unquote(parts[1]);