Peter Stuge [off-list ref] writes:
Okey. I thought subject together with change would be clear enough. :)
quoted
Explanation of what you are fixing is totally lacking.
The subject sums it up, if briefly.
... Sorry, that is not what I meant.
You don't have to explain these to *me* specifically as a response to this
thread. What I meant was that your patch should have these necessary
descriptions in its proposed commit log message.
Junio C Hamano wrote:
quoted
I thought subject together with change would be clear enough. :)
quoted
Explanation of what you are fixing is totally lacking.
The subject sums it up, if briefly.
... Sorry, that is not what I meant.
You don't have to explain these to *me* specifically as a response
to this thread. What I meant was that your patch should have these
necessary descriptions in its proposed commit log message.
IMO not so neccessary if one knows a little web and javascript, which
is probably likely for a gitweb change..
It's a simple fix of links broken by manual URI manipulation that
didn't consider fragments. Is the subject description really not
enough?
//Peter
Am 9/27/2011 0:28, schrieb Peter Stuge:
Junio C Hamano wrote:
quoted
You don't have to explain these to *me* specifically as a response
to this thread. What I meant was that your patch should have these
necessary descriptions in its proposed commit log message.
IMO not so neccessary if one knows a little web and javascript, which
is probably likely for a gitweb change..
It's a simple fix of links broken by manual URI manipulation that
didn't consider fragments. Is the subject description really not
enough?
No, it is not. The target audience of a commit message are people like I
am. I do know a bit of Perl, and a bit of Javascript; I know how an URL is
structured; I would find my way through the gitweb code if the need
arises. But I am not an expert in any of these areas.
The subject alone is not sufficient because I do not know for sure what an
"URI fragment" is or what role line numbers in gitweb's links play. The
explanations and examples you gave in a later email were very
enlightening, and they would be very helpful if *I* am forced to hack
gitweb, and if I need to understand why this particular change was good.
Finding the right balance between verbosity and terseness needs practice,
but to write *no* justification is practically always wrong.
-- Hannes
Hey!
Johannes Sixt wrote:
quoted
It's a simple fix of links broken by manual URI manipulation that
didn't consider fragments. Is the subject description really not
enough?
No, it is not. The target audience of a commit message are people like I
am. I do know a bit of Perl, and a bit of Javascript; I know how an URL
is structured; I would find my way through the gitweb code if the need
arises. But I am not an expert in any of these areas.
The subject alone is not sufficient because I do not know for sure what
an "URI fragment" is or what role line numbers in gitweb's links play.
I shall continue playing advocatus diaboli only a little longer.
The explanations and examples you gave in a later email were very
enlightening, and they would be very helpful if *I* am forced to hack
gitweb, and if I need to understand why this particular change was good.
On the other hand you're just one quick google search on uri fragment
away from the same enlightenment, and relying on terminology saves on
unneccessary redundance.
Lorelei: That's repetitive
Rory: ..and redundant.
Lorelei: That's repetitive
Rory: ..and redundant.
(SCNR the pop culture reference! :)
Finding the right balance between verbosity and terseness needs
practice,
I disagree, but I agree with you if we qualify that a little. The
right balance is a matter of subjective review, so the only way it
can be practiced with relevance is by actually working with the same
reviewers for a while, to learn what they consider right.
It can absolutely not be practiced out of context, ie. with different
peers. No later than the day before I sent this patch I wrote a
welcome mail in another open source project, to a new contributor,
where one bit was about commit messages.
http://marc.info/?l=openocd-development&m=131698532523018
"* Write a top quality commit message, technically and logically
...
As for the logical quality, it is important to write the first line
description of the change so that it makes sense for someone who
knows nothing at all about the code, since this is used in list
views, and since the background for this code and for why this change
was done the way it was done comes only in the later lines, which may
not be available from where that list view is. ... Keep it
high level, clear and simple. Writing this one line is not
neccessarily easy."
I of course also try to practise exactly this, but it's difficult to
know what reviewers expect to be fed, or how much verbosity they
prefer. :) I tend to prefer as much useful information as possible in
the first line, while keeping it ideally <60 chars. Many times I find
it to be enough.
but to write *no* justification is practically always wrong.
I disagree strongly that I wrote no justification. I agree that it
was not verbose. I'm sorry that this is a problem. I'm personally
allergic to redundancy such as the commit message Jakub wrote, I
think it's not only reasonable but also desirable to avoid that.
Maybe gitweb is a special case in git.git though, I don't know, but
I'm a little surprised. :)
Anyway, I'm more than happy to write a more verbose message for you!
//Peter
Some javascript code will run in the browser onLoad and signals back to
gitweb that the browser can actually do javascript.
The code adds [?;]js=1 into the URL of all links on the page. The code
always added [?;]js=1 to the end of links, which is wrong when links
contain a URI fragment, such as links directly to a line in a blob:
..?p=repo.git;a=blob;f=file#l123
In this case, [?;]js=1 must be added before the hashmark.
Signed-off-by: Peter Stuge <redacted>
---
gitweb/static/js/javascript-detection.js | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/gitweb/static/js/javascript-detection.js b/gitweb/static/js/javascript-detection.js
index 93dd2bd..fa2596f 100644
--- a/gitweb/static/js/javascript-detection.js
+++ b/gitweb/static/js/javascript-detection.js
@@ -16,7 +16,7 @@
* and other reasons to not add 'js=1' param at the end of link
* @constant
*/
-var jsExceptionsRe = /[;?]js=[01]$/;
+var jsExceptionsRe = /[;?]js=[01](#.*)?$/;
/**
* Add '?js=1' or ';js=1' to the end of every link in the document
@@ -33,9 +33,9 @@ function fixLinks() {
var allLinks = document.getElementsByTagName("a") || document.links;
for (var i = 0, len = allLinks.length; i < len; i++) {
var link = allLinks[i];
- if (!jsExceptionsRe.test(link)) { // =~ /[;?]js=[01]$/;
- link.href +=
- (link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1';
+ if (!jsExceptionsRe.test(link)) {
+ link.href = link.href.replace(/(#|$)/,
+ (link.href.indexOf('?') === -1 ? '?' : ';') + 'js=1$1');
}
}
}--
1.7.4.1.343.ga91df.dirty