Re: [PATCH v4 0/7] git send-email suppress-cc=self fixes
On Sun, Jun 09, 2013 at 04:25:11PM -0700, Junio C Hamano wrote:
Michael S. Tsirkin m...@redhat.com writes:
With respect to this, and a bit off-topic, what's
the best way to revise patch series?
What I did, given series in patchvN-1/:
rm -fr patchvN #blow away old directory if there
# otherwise I get two copies of patches if I renamed any
Not needed with recent git format-patch -v4 option.
Unless I rerun with same vX :(
Would it make sense for it to check for vX existance and fail?
Same without -vX, when 000X exists ...
Could be an option.
git branch|fgrep '*'
# Figure out on which branch I am, manually specify the correct
upstream I'm tracking,
# otherwise I get a ton of unrelated patches.
git-prompt with PS1 you do not need this either.
grep serves just as well but
I still need to copy it to the next line manually...
I vaguely remember there was some way to say
head of the remote I am tracking - but I could not find it.
Where are all the tricks like foo^{} documented?
I tried fgrep '{}' Documentation/*txt and it only returned
git-show-ref.txt which isn't really informative ...
Additionally, or alternatively, would it make sense for git format-patch
to format the diff against the tracking branch by default?
git format-patch --cover --subject-prefix='PATCH vN' -o patchvN
origin/master..
Again, git format-patch -v4 -o mt-send-email will deposit the new
ones alongside the older round.
vi patchvN/* patchvN-1/*
Same (i.e. vi mt-send-email/v*--*.txt).
I still need to copy subject, Cc list and blurb to the next line manually.
Now that there's a concept of revisions,
maybe git format-patch -v4 could copy the text
and subject from v3?
--
MST
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: Revert 77c1305 and 3c3b46b The core of the argument seems to be that __git_complete_revlist_file() is a misleading name for the completion done by archive and ls-tree, but __git_complete_file() is somehow a less misleading name? Irrespective of what the valid completions of the various commands are, I want to know which completion function will be _used_ when I'm reading the script. And that is __git_complete_revlist_file(). To me, it looks like mega-bikeshedding; a huge amount of time and effort going behind a non-functional variable rename (and the best part? the rename isn't getting us a better name; just a historical name). But whatever. To me the most important part is that we have two separate functions that are used consistently by how the completion is to be done for their users. The complete-file variant can then lose the A..B range completion, and then be given a better name than FILE to express what it does if somebody can find one. When it happens, the same better name should be used to rename complete-revlist-FILE by replacing the FILE part. I initially thought that FILE referred to the pathspecs (i.e. the last part in log rev file), and felt it was strange to call it FILE. Perhaps that (i.e. it not being pathspec) is what you find it misnamed). But it turns out that in the context of these functions it refers to what users consider paths in objects stored in the object database (as opposed to working tree paths). That is what ls-tree would take (i.e. tree-ish and tree-ish:path). And I do not offhand think of a better name myself to strongly oppose to using the word FILE to refer to what it does. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] commit-queue: LIFO or priority queue of commits
Jeff King p...@peff.net writes:
It may be worth looking again for other places to use this over
commit_list, but even the caller you are introducing here justifies its
presence.
The next candidate is paint-down-to-common, probably.
Also, I wrote some basic tests to cover the priority queue as a unit. I
can rebase them on your commit if you are interested.
It would be great.
A few comments on the code itself:
+void commit_queue_put(struct commit_queue *queue, struct commit *commit)
Is it worth making this struct commit * a void pointer, and handling
arbitrary items in our priority queue? The compare function should be
the only thing that dereferences them.
I do not have any non-commit priority queue use in mind, but I do not
think it adds any complexity in this case.
I didn't either (and still I don't think of one), but I agree that
the implementation can be reused for pq of any type, as long as it
is a pointer to struct.
+/* Bubble up the new one */
+for (ix = queue-nr - 1; ix; ix = parent) {
+parent = (ix - 1) / 2;
+if (compare(queue-array[parent], queue-array[ix],
+queue-cb_data) 0)
+break;
In my implementation, I stopped on compare() = 0. It is late and my
mind is fuzzy, but I recall that heaps are never stable with respect to
insertion order, so I don't think it would matter.
It would matter in the sense that we cannot replace linked-list, if
the caller wants stability. It is more like we cannot do anything
about it than it would not matter.
We can make each queue element a pair of pointer to payload,
insertion counter, and tiebreak using the insertion order, if the
callers want the same stability as linked-list implementation, but
I tend to think it really matters.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] sort-in-topological-order: use commit-queue
Jeff King p...@peff.net writes: The performance enhancement of the priority queue came from replacing commit_list_insert_by_date calls with insertion into a queue. That drops O(n^2) behavior on the linked-list down to O(n log n), as we have n insertions, each causing an O(log n) heapify operation. Yes. Around the same time, though, René wrote the linked-list merge sort that powers commit_list_sort_by_date. And topo-sort learned to do O(1) insertions into the unsorted list, and then one O(n log n) sort. Yes, but that only affects the sort the work queue in date order before entering the main loop, and maintenance of work queue as we dig along still is find the place to put this in the date-order sorted linked list, no? So your results are exactly what I would expect: the time should be about the same (due to the same complexity), but the memory is used more compactly (array of pointers instead of linked list of pointers). I've been disturbed every time I saw the commit_list insertion function that does a small allocation which will be freed fairly often and have been wondering if we can rewrite it with custom slab allocator, but not using linked list where we do not have to feels like a better solution to that issue, and use of pqueue may be a right direction to go in. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/7] git send-email suppress-cc=self fixes
On Mon, Jun 10, 2013 at 09:53:24AM +0300, Michael S. Tsirkin wrote:
I vaguely remember there was some way to say
head of the remote I am tracking - but I could not find it.
Where are all the tricks like foo^{} documented?
gitrevisions(7) is what you're looking for here.
In this case I think you want '@{upstream}' or its short form '@{u}'.
I tried fgrep '{}' Documentation/*txt and it only returned
git-show-ref.txt which isn't really informative ...
'{' and '}' need to be escaped in AsciiDoc so you have to grep for
'\\{\\}'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] log: --author-date-order
Jeff King p...@peff.net writes: I'm not excited about introducing yet another place that parses commit objects (mostly not for correctness, but because we have had inconsistency in how malformed objects are treated). It is at least using split_ident_line which covers the hard bits. I wonder how much slower it would be to simply call format_commit_message to do the parsing. The thought certainly crossed my mind, not exactly in that form but more about splitting the machinery used in pretty.c into a more reusable form. The result of my attempt however did not become all that reusable (admittedly I didn't spend too much brain cycles on it), so I punted ;-). The record_author_date function assumes that commit-buffer is valid (i.e., not NULL). We seem to assume that the commits are parsed already (for looking at parents, and at the committer date). I thought that the latter is warranted, as the function worked on the output of limit_list(), and by the time limit_list() finishes, everything relevant must have been parsed already. But you are right. The commit-buffer may no longer be there, and the --author-date-order option needs to read the object again in this codepath. That would be in line with what --pretty/format would do, I guess. Or we could extend parse_commit() API to take an optional commit info slab to store not just author date but other non-essential stuff like people's names, and we arrange that extended API to be triggered when we know --author-date-order is in effect? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #03; Thu, 6)
Junio C Hamano wrote: But it turns out that in the context of these functions it refers to what users consider paths in objects stored in the object database (as opposed to working tree paths). That is what ls-tree would take (i.e. tree-ish and tree-ish:path). And I do not offhand think of a better name myself to strongly oppose to using the word FILE to refer to what it does. __git_complete_treeish(). Write a new patch with a proper comment saying why it is aliased to __git_complete_revlist_file(). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rm: better error message on failure for multiple files
Once again, thanks a lot your feedback, we appreciate it a lot!
Le 2013-06-08 15:51, Ramkumar Ramachandra a écrit :
Mathieu Lienard--Mayor wrote:
@@ -170,30 +175,47 @@ static int check_local_mod(unsigned char
*head, int index_only)
* intent to add entry.
*/
if (local_changes staged_changes) {
- if (!index_only || !(ce-ce_flags
CE_INTENT_TO_ADD))
- errs = error(_('%s' has staged
content different
-from both the file and
the HEAD\n
-(use -f to force
removal)), name);
+ if (!index_only || !(ce-ce_flags
CE_INTENT_TO_ADD)) {
+ strbuf_addstr(files_staged, \n
);
Ouch. Wouldn't a string-list be more appropriate for this kind of
thing?
Matthieu Moy told me string-list would be better aswell, so we're gonna
change it.
--
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [IGNORE] Implement 'git rebase' in ruby
Felipe Contreras wrote: git-rebase.rb | 2056 + 1 file changed, 2056 insertions(+) create mode 100755 git-rebase.rb I suggest putting this in contrib/ and cooking it. As usual, my mantra is: let the patches decide what to do. I'll help review and improve this soon. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/7] git send-email suppress-cc=self fixes
Michael S. Tsirkin m...@redhat.com writes:
Not needed with recent git format-patch -v4 option.
Unless I rerun with same vX :(
Would it make sense for it to check for vX existance and fail?
Same without -vX, when 000X exists ...
Could be an option.
Oh, instead of exact -v$N, trigger it with -v auto or something?
Sounds like a good addition.
And instead of ***BLURBHERE*** placeholder, text from old round
could be copied as a new placeholder. I do not offhand think that
needs much thought about compatibility but maybe there are people
who trained their editors or scripts to find the known placeholder
string and edit it? I dunno. It certainly sounds like a sensible
thing to do to carry as much information forward from the older
round if/when we know which one corresponds to which.
Discussions and patches welcome.
git branch|fgrep '*'
# Figure out on which branch I am, manually specify the correct
upstream I'm tracking,
# otherwise I get a ton of unrelated patches.
git-prompt with PS1 you do not need this either.
grep serves just as well but
I still need to copy it to the next line manually...
I vaguely remember there was some way to say
head of the remote I am tracking - but I could not find it.
Do you mean @{upstream}?
Where are all the tricks like foo^{} documented?
Documentation/revisions.txt?
Additionally, or alternatively, would it make sense for git format-patch
to format the diff against the tracking branch by default?
Meaning git format-patch @{u} without saying anything about @{u}?
I am not sure if we want to go that far, but it certainly is worth a
thought.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Le 2013-06-08 16:01, Ramkumar Ramachandra a écrit : Mathieu Lienard--Mayor wrote: As an example, the message: error: 'foo.txt' has changes staged in the index (use --cached to keep the file, or -f to force removal) would look like, with advice.rmHints=true: error: 'foo.txt' has changes staged in the index Um, have you switched the true with false? advice.* variables are true by default, and I turn off all of them. Whoops, my bad, I obviously meant false. Also, I think you can extend this to also remove add-advice. Why would someone want to turn off advice from rm, but not add? (Unsure about this) I'm not so sure i understand. Do you mean rmHints should deactivate addHints aswell, or do you mean that since we're introducing rmHints it would be natural to introduce addHints ? Similarly to advice.*, advice.rmHints has been added to the config variables. By default, it is set to false, in order to keep the messages the same as before. When set to true, advice are no longer included in the error messages. Ugh, why this roundabout-passive-past tone? Use imperative tone like this: Sorry about that, we'll work on it. Introduce advice.rmHints to control the whether to display advice when using 'git rm'. Defaults to true, preserving current behavior. -- Mathieu Liénard--Mayor, 2nd year at Grenoble INP - ENSIMAG (+33)6 80 56 30 02 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Mathieu Liénard--Mayor wrote: I'm not so sure i understand. Do you mean rmHints should deactivate addHints aswell, or do you mean that since we're introducing rmHints it would be natural to introduce addHints ? More the latter, but I'm tilting towards addRmHints (or something) which affects both add and rm hints. Sorry about that, we'll work on it. Nothing to be sorry about. You're doing good work, and we're helping you make it even better :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] status: introduce status.short to enable --short by default
El 2013-06-08 17:25, Ramkumar Ramachandra escribió:
Jorge Juan Garcia Garcia wrote:
Some people always run 'git status -s'.
The configuration variable status.short allows to set it by default.
Good feature.
@@ -1112,6 +1112,15 @@ static int git_status_config(const char *k,
const char *v, void *cb)
s-submodule_summary = -1;
return 0;
}
+ if (!strcmp(k, status.short)) {
+ if (!v)
+ return config_error_nonbool(k);
+ if (git_config_bool(k,v)) {
+ status_format = STATUS_FORMAT_SHORT;
+ wt_shortstatus_print(s);
+ }
+ return 0;
+ }
Incorrect. This is the wrong place to use config_error_nonbool():
this is very much a bool, and a [status] short in ~/.gitconfig
should not error out (all boolean variables behave in the same
manner). When in doubt, consult config_error_nonbool(); there's
clearly a comment stating:
Ok. We will change it.
/*
* Call this to report error for your variable that should not
* get a boolean value (i.e. [my] var means true).
*/
Also, why are you calling wt_shortstatus_print() here, instead of
returning control to cmd_status(), which is going to do it anyway?
Yes, it's obviously a mistake.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Ramkumar Ramachandra artag...@gmail.com writes: Mathieu Liénard--Mayor wrote: I'm not so sure i understand. Do you mean rmHints should deactivate addHints aswell, or do you mean that since we're introducing rmHints it would be natural to introduce addHints ? More the latter, but I'm tilting towards addRmHints (or something) which affects both add and rm hints. I don't see why add and rm hints should be correlated, or I don't have the same advice as you in mind. $ git add foo.txt The following paths are ignored by one of your .gitignore files: foo.txt Use -f if you really want to add them. fatal: no files added $ git rm foo.txt error: 'foo.txt' has changes staged in the index (use --cached to keep the file, or -f to force removal) Both have completely different meanings: the first is about .gitignore, and the second about not loosing data. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Matthieu Moy wrote: I don't see why add and rm hints should be correlated, or I don't have the same advice as you in mind. Both have completely different meanings: the first is about .gitignore, and the second about not loosing data. Right, my bad. Please continue with rmHints, and optionally write an addHints while at it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 09/28] git-remote-mediawiki: Change the behaviour of a split
Célestin Matte celestin.ma...@ensimag.fr writes: A split ' ' is turned into a split / /, which changes its behaviour: the old method matched a run of whtespaces (/\s*/) It case there's a v4: whtespaces - whitespaces. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Felipe Contreras felipe.contre...@gmail.com writes: On Sun, Jun 9, 2013 at 4:39 PM, Junio C Hamano gits...@pobox.com wrote: One example of killing the entire thread is when I see This patch will not be applied by Felipe in a thread started with his patch. I understand that it is his way to say this patch is retracted without having to explicitly say that he now understands that reviews showed why the patch was wrong or that he thanks the reviewer for enlightening him. You are wrong. There's nothing wrong with the patch. ... I thought you understood that code should speak, but apparently you don't. That is exactly the point Peff raised (and I agreed with), isn't it? Bad behaviour (being difficult to work with) has consequences. E.g. convincing people that it is not worth their time interacting with you, especially when there are better things to do like tending to other topics, and you lose the chance to show that your patches are good when they indeed are (I don't even know if these patches in question are good, and I am not going to find out). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style
Célestin Matte celestin.ma...@ensimag.fr writes:
@@ -1285,8 +1285,7 @@ sub get_mw_namespace_id {
# Look at configuration file, if the record for that namespace
is
# already cached. Namespaces are stored in form:
# Name_of_namespace:Id_namespace, ex.: File:6.
- my @temp = split(/\n/, run_git(config --get-all remote.
- . $remotename
..namespaceCache));
+ my @temp = split(/\n/, run_git(config --get-all
remote.${remotename}.namespaceCache));
I tend to prefer the former, as it avoids long lines ( 80 columns)
@@ -1339,8 +1338,7 @@ sub get_mw_namespace_id {
# Store explicitely requested namespaces on disk
if (!exists $cached_mw_namespace_id{$name}) {
- run_git(config --add remote.. $remotename
- ..namespaceCache \. $name .:. $store_id .\);
+ run_git(qq(config --add remote.${remotename}.namespaceCache
${name}:${store_id}));
Same.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] push: make upstream, simple work with pushdefault
Junio C Hamano wrote:
I am not sure what you mean by artificial.
By artificial, I mean that the precondition is absolutely unnecessary
for the code following it to work. The precondition was introduced in
a separate commit, specifically denying one usecase because the author
(you) thought that made sense.
If you have push.default set upstream or simple, then a push run
while on the branch 'foo' will figure out what happens when you do a
fetch by looking at 'branch.foo.merge' to find the branch we are set
to integrate from 'branch.foo.remote' remote. The simple further
says that branch name must be the same as 'foo'.
I understand this perfectly well, as evidenced by the tests I've
written out in 4/4.
And that is what setup_push_UPSTREAM() is designed to do. Rejecting
a call that breaks the precondition is perfectly the right thing to
do: if you set to push to upstream and if you are trying to push
to a different remote, for example.
The triangle topic changed the precondition without updating the
logic to check it, which was the bug, not the original check.
Did I claim otherwise? :)
The topic is about fixing a bug introduced by rr/triangle.
I actually am OK with 'upstream' that rejects triangular, while
making 'simple' do something different [*1*].
Okay, so you haven't outlined a solution either. Like I said in the
cover-letter, I've spent hours breaking my head and can't figure out a
better solution. The bigger problem is that upstream/ simple were
designed with only central workflows in mind. How do we deal with it
now?
Yes, upstream/ simple work only when @{u} resolves, and I haven't
changed this (because we don't have a well-defined meaning for what
branch.name.merge without a branch.name.remote means). My
argument is very simple: no push.default mode should not dictate the
push destination; only the push refspec. What makes sense to the user
is an upstream/ simple that works as expected with pushdefault: do the
tests I've outlined in 4/4 not make sense? *scratches head*
I don't understand why upstream/ simple should _not_ push to a
different destination from the one being merged from. I'll repeat:
push source/ destination is orthogonal to push refspec, and
push.default modes dictate the refspec.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/28] Follow perlcritic's recommandations
Eric Sunshine sunsh...@sunshineco.com writes: On Sun, Jun 9, 2013 at 6:22 PM, Célestin Matte celestin.ma...@ensimag.fr wrote: Changes with v2: - Remove patch [02/22] about using the Readonly module - Split commit [07/22] into 5 different ones This was easier to review after being split. Thanks. - Split commit [14/22] into 2 different ones - Patch [17/22] was *not* split: tell me if it is necessary [now patch 22/28] You, Matthieu, and Junio should decide, but I again found it time-consuming and onerous to review with all the changes mashed together. I agree that it would have been better to split the patches in v1, but now that we've already spent time reviewing it, it seems unecessary to spend more time splitting and re-reviewing. I went through the series once more, all my remarks are minor. I'm OK with the series as it is (i.e. perhaps it's time to say stop the bikeshedding and start coding real stuff ;-) ). As a reminder: Celestin is in a school project that ends in a week. The goal is both to be productive and to learn stuff. In any case, thanks a lot for your review, Eric. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/7] git send-email suppress-cc=self fixes
On Mon, Jun 10, 2013 at 08:29:31AM +0100, John Keeping wrote:
On Mon, Jun 10, 2013 at 09:53:24AM +0300, Michael S. Tsirkin wrote:
I vaguely remember there was some way to say
head of the remote I am tracking - but I could not find it.
Where are all the tricks like foo^{} documented?
gitrevisions(7) is what you're looking for here.
I see. And git(1) actually points to it.
Thanks!
In this case I think you want '@{upstream}' or its short form '@{u}'.
I tried fgrep '{}' Documentation/*txt and it only returned
git-show-ref.txt which isn't really informative ...
'{' and '}' need to be escaped in AsciiDoc so you have to grep for
'\\{\\}'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bad attitudes and problems in the Git community (was: Re: [PATCH 2/2] Move sequencer to builtin)
On 06/10/2013 07:15 AM, Felipe Contreras wrote: On Sun, Jun 9, 2013 at 6:40 PM, Stefano Lattarini stefano.lattar...@gmail.com wrote: I do accuse Felipe's *attitude* to bring on and nourish such unpleasantness toxicity. His technical merits and the possible qualities of his patches do *nothing* to remove or quell such issues. How convenient to accuse me I accuse your *attitude*, which IMHO is now even damaging your code (that is, the acceptance of your code on the part of the community). and not the others who have as much fault if not more. Sorry, this is just your opinion. Mine, by observing the proceeding from the outside, is different: my opinion is that your attitude is the problem. You need two sides to have an argument. I disagree. Unless you mean than, whenever a part behaves in a hostile and aggressive way, the other part should just silently knuckle under. The difference is; I did actually send code. Code that is good, code that works, and code that users need. As I said, as a *user* (since I'm definitely not a Git developer in any sense of the word), I also need a calm and constructive environment in the community. Or are you interested only in users that can benefit from things you are good at? Regards, Stefano -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: remove ancient ensure colon in COMP_WORDBREAKS workaround
First things first: Junio, please don't pick up this patch. On Sun, Jun 09, 2013 at 05:09:54PM -0700, Jonathan Nieder wrote: SZEDER Gábor wrote: Fedore 9 shipped the gvfs package with a buggy Bash completion script: it removed the ':' character from COMP_WORDBREAKS, thereby breaking certain features of git's completion script. We worked this around in db8a9ff0 (bash completion: Resolve git show ref:pathtab losing ref: portion, 2008-07-15). The bug was fixed in Fedora 10 and Fedora 9 reached its EOL on 2009-07-10, almost four years ago. It's about time to remove our workaround. Nice! I had wondered what that workaround was about but never ended up digging into it. Meh, not nice at all. Searching for COMP_WORDBREAKS in /etc/bash_completion.d/* finds similar breakage (removal of '=' and '@') in the npm completion script, but nothing involving colon. So this looks like a good change. And apparently I have a completion script called axi-cache (from package apt-xapian-index) which removes ':' [1]. However, it doesn't remove the ':' upon loading the script but the removal is done in the completion function, i.e. it takes effect only when the user actually attempts to complete its options. I never use axi-cache, whatever the hell it might be, so I didn't notice. Unfortunately, with my above patch applied I get this in a new shell: $ git show master:qTAB $ git show master:quote.TAB master:quote.c master:quote.h $ axi-cache TAB again depends help madison more policyrdepends searchshow showpkg showsrc $ git show master:qTAB $ git show quote.TAB quote.c quote.h quote.o Not good. I don't have the npm package, but manually removing '=' from COMP_WORDBREAKS leads to similar breakage with e.g. 'git log --pretty='. Neither this axi-cache not npm completion script comes from the bash-completion project. Apparently some developers providing completion scripts for their projects lack the necessary expertise to consider the effects their script might have on other completion scripts. Perhaps distributions should adopt a policy not to allow completion scripts messing with COMP_WORDBREAKS, dunno. Anyway. Although the completion script in Fedora's gvfs package might be fixed, there are other completion scripts making the same mistake, so I'm afraid we should keep the workaround and drop this patch. Moreover, we should even consider extending our workaround by adding back '=' to COMP_WORDBREAKS, too. Oh, well. [1] - This might not be accurate nowadays, as my system is a bit oldish... Best, Gábor -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] push: make upstream, simple work with pushdefault
Ramkumar Ramachandra artag...@gmail.com writes: I don't understand why upstream/ simple should _not_ push to a different destination from the one being merged from. I'll repeat: push source/ destination is orthogonal to push refspec, and push.default modes dictate the refspec. That is where we differ. You keep repeating that without justifying why reusing the branch upstream would have chosen in a non-triangular workflow is a good idea in the first place. Let's step back a bit and think. In a workflow where we both agree that upstream mode makes sense, i.e. central repository workflow, we may have a triangle topic in our shared repository, and I may have my own topic forked from it (i.e. set to fetch that triangle and integrate my work with it). The topic is to work on one aspect of triangular workflow, e.g. pushbranch (i.e. I am not worried about how we choose which remote to push to, but interested in what branch at that remote is updated). It makes sense for me to push the result back to the upstream, i.e. triangle branch at the shared remote where I forked from, to collaborate with other people (e.g. you) whose shared purpose is to improve the triangular workflow. You may be doing the same, and your local branch may be called triangle (bag of various triangular workflow improvements), or pushremote (a specific subtopic). In either case, you push to upstream to update triangle. What should happen in a triangular workflow, where I may be fetching and integrating from your repository to work on triangular workflow in general (let's say that you are the owner of that topic) to advance the support for that workflow. The topic in your repository may be called triangle to improve triangular workflow in general, and again my branch would be pushbranch to improve one specific aspect of it. As we are using triangular workflow, the place *I* push to is under my control, not the shared maintainer one or your repository, and I'll be requesting you to pull that topic from me. Now think _why_ I renamed the branch on my end, i.e. not calling that branch in question triangle that is the blanket name for the collective effort but calling it with a more specific name pushbranch, in the first place. It is to clarify that my part of the effort is that particular subarea of the topic. Wouldn't it be a lot more sensible to use that more specific name when publishing, rather than triangle name, and ask you to pull that branch that describes what I concentrated on? Push destination and fetch source, when they are different, refer to different repositories by definition, and they are in different administration domains. Why should the name used to publish my work which is a specialization of what I built on (i.e. your triangle) lose the specialization from its name when I push it out? We can afford not to lose information in the branch name. The difference between the shared repository case and triangular workflow primarily comes from the fact that the final integration happens at a different place. In the shared repository case, the integration happens in each participant's local repository and push updates the shared history at the known name. If my pushbranch specialization diverted from the overall triangle effort, I would first integrate others work and then update the remote, and at that point, what I am pushing is no longer an isolated specialized effort, but the result of its integration into a larger topic. And once the more specialized topic is integrated into a more generic branch it was forked from, it makes sense to update that more generic branch at the shared repository. In the triangular case, however, you as the triangle topic owner has the choice to fetch and integrate from me or not to do so. And the integration happens when you decide to fetch and integrate. There is no reason whatsoever to lose the more specific name until that integration happens into your (meaning: the owner of the more general triangle topic) bag of various triangular improvements topic. Imagine the case I forked two branches from your triangle topic and pushing them to my publishing repository using the triangular workflow. Why should I attempt to update a single triangle branch with both of these topics? The name under which the local branch is published needs a sensible default (when branch.$name.push is not specified), and I agree that you would get the name of the branch my work was forked from if you reuse the upstream code. I am saying that it does not necessarily give us a good default. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] test: improve rebase -q test
On Sun, Jun 09, 2013 at 03:41:54PM -0500, Felipe Contreras wrote: There will not be a need for test_string_must_be_empty() just like there's no need for test_string_cmp(). Actually, if there were a test_string_cmp(), that would be the test helper function I used most often. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style
Le 10/06/2013 02:50, Eric Sunshine a écrit :
Given this patch's intention to use ${} within strings, should this be
${credential{username}}?
(I don't have a preference, but it's a genuine question since it's not
clear if this was an oversight or intentional.)
The answer is simple: I didn't know the exact syntax, so I didn't bother
doing it.
The whitespace-only change to line my $res = do { is effectively
noise. The reviewer has to stop and puzzle out what changed on the
line before continuing with review of the remaining _real_ changes. It
is a good idea to avoid noise changes if possible.
In this particular case, it's easy to avoid the noise since the
trailing space on that line could/should have been removed in patch
18/28 when the statement was split over multiple lines.
Actually, I noticed this but didn't find what the difference between the
two lines was. I assumed git was making some kind of mistake - but eh,
it seems git is never wrong :)
local $/ = undef;
$git
};
@@ -475,26 +475,26 @@ sub download_mw_mediafile {
return $response-decoded_content;
} else {
print STDERR Error downloading mediafile from :\n;
- print STDERR URL: $download_url\n;
- print STDERR Server response: . $response-code . .
$response-message . \n;
+ print STDERR URL: ${download_url}\n;
+ print STDERR 'Server response: ' . $response-code . q{ } .
$response-message . \n;
To meet the goals of this patch, would you want to do this instead?
Server response: @{[$response-code]} @{[$response-message]}\n;
Whether this is easier or more difficult to read is a matter of
opinion. (Again, this is a genuine question rather than a show of
preference on my part.)
Same as above, I tried to change it but didn't know the exact syntax, so
I gave up.
--
Célestin Matte
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style
Le 10/06/2013 10:37, Matthieu Moy a écrit :
Célestin Matte celestin.ma...@ensimag.fr writes:
@@ -1285,8 +1285,7 @@ sub get_mw_namespace_id {
# Look at configuration file, if the record for that namespace
is
# already cached. Namespaces are stored in form:
# Name_of_namespace:Id_namespace, ex.: File:6.
-my @temp = split(/\n/, run_git(config --get-all remote.
-. $remotename
..namespaceCache));
+my @temp = split(/\n/, run_git(config --get-all
remote.${remotename}.namespaceCache));
I tend to prefer the former, as it avoids long lines ( 80 columns)
The 80-columns rule is already broken in *many* places in this file.
I know this is not a valid reason to do it more often, but in my opinion
the second version is easier to read (it's easy to miss the dots in the
first version), so we would gain from the change.
--
Célestin Matte
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rm: better error message on failure for multiple files
Le 08/06/2013 10:33, Mathieu Lienard--Mayor a écrit : + if (files_staged.len) + errs = error(_(the following files have staged content +different from both the\nfileand the HEAD:%s\n +(use -f to force removal)), files_staged.buf); Typo here: fileand - file and -- Célestin Matte -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mingw: make mingw_signal return the correct handler
On Mon, Jun 10, 2013 at 7:48 AM, Johannes Sixt j.s...@viscovery.net wrote: From: Erik Faye-Lund kusmab...@gmail.com Returning the SIGALRM handler for SIGINT is not very useful. Signed-off-by: Erik Faye-Lund kusmab...@gmail.com Signed-off-by: Johannes Sixt j...@kdbg.org --- Am 6/7/2013 16:20, schrieb Erik Faye-Lund: On Fri, Jun 7, 2013 at 3:07 PM, Johannes Sixt j.s...@viscovery.net wrote: BTW, isn't mingw_signal() bogus in that it returns the SIGALRM handler even if a SIGINT handler is installed? Yep, that's a bug. Thanks for noticing. This is your patch to address it. I've pushed out a branch here that tries to address these issues, but I haven't had time to test them. I'll post the series when I have. In the mean time: https://github.com/kusma/git/tree/win32-signal-raise Concerning the other two patches: * SIGINT: perhaps handle only the SIG_DFL case (for the exit code) and forward all other cases to MSVCRT? Perhaps. I'll have to think a bit about it, but it might very well be the sanest approach, as long as it doesn't break git_terminal_prompt (which was the reason for the change in the first place). I can't of the top of my head understand why it should, though. * SIGTERM: it papers only over a general issue and should be dropped. Fair enough. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t0005: skip signal death exit code test on Windows
On Mon, Jun 10, 2013 at 7:30 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 6/9/2013 22:31, schrieb Junio C Hamano: Jeff King p...@peff.net writes: I'm a little negative on handling just SIGTERM. That would make the test pass, but does it really address the overall issue? To me, the usefulness is having exit values with consistent meanings. Yes. Unless the goal is to give Windows port pratically the same signal semantics as ports on other platforms, I do not think special casing SIGTERM (unless it is a very common signal on Windows and others are unlikely to be useful) buys us much. I'm thinking the same. And, no, SIGTERM is not very common on Windows. I have no strong feelings on SIGTERM, but my knee-jerk reaction is the same. AFAIK, the only issue we've seen with it has been this one, which is synthetic. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] push: make upstream, simple work with pushdefault
Junio C Hamano wrote:
The name under which the local branch is published needs a sensible
default (when branch.$name.push is not specified), and I agree that
you would get the name of the branch my work was forked from if you
reuse the upstream code. I am saying that it does not necessarily
give us a good default.
See, that's the problem: push.default is simply a default refspec to
push to for all branches, that can be _overridden_ by
branch.name.push. Getting an override is not going to solve the
problem we are facing: what to do when branch.name.push is
unspecified? Fall back to push.default, right?
Now think _why_ I renamed the branch on my end, i.e. not calling
that branch in question triangle that is the blanket name for the
collective effort but calling it with a more specific name
pushbranch, in the first place.
Look, it's very simple. upstream was built to support the case when
the local branch name is different from the remote branch name, but it
was too specialized for central workflows. How do we extend it for
triangular workflows?
Just like we introduced branch.name.pushremote to override
branch.name.remote, we get branch.name.push to override
branch.name.merge. If branch.name.pushremote is unset, where do
the pushes go? branch.name.remote. If branch.name.push is
unspecified, what is the refspec to be pushed? branch.name.merge
(when push.default = upstream) [*1*].
What does this mean? I publish the branch triangle on ram (what my
local branch is called or what push.default I use is irrelevant). You
have a branch called pushremote with branch.pushremote.remote set to
ram, remote.pushdefault set to junio, branch.pushremote.merge set to
refs/heads/triangle, and push.default set to upstream.
# on jc's machine; on branch pushremote
$ git pull
# integrates changes from ram's triangle just fine
$ git push
# publishes the branch as triangle on remote junio
I rewrite my branch, and you have 4 commits based on my branch:
$ git rebase --onto @{u} @~4
Perfect.
The only limitation is that you don't choose the branch name on junio.
When we get branch.pushremote.push, you'll be able to set it to
refs/heads/pushremote, making push.default inconsequential. Done.
[Footnotes]
*1* remote.pushdefault overrides branch.name.remote, while
push.default will be overridden by a future branch.name.push. Not
exactly elegant, is it?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/15] Towards a more awesome git branch
[-CC: Duy, since he has left the community] Junio: since Duy is no longer around to guide us, I will rely on your guidance. Duy Nguyen wrote: I'm starting to think this is a half-baked solution. It hides problems, for example commit placeholders should produce empty string, not the literal placeholders. Why should they produce empty strings? Aren't they equivalent to invalid placeholders? Doing that from a callback is really ugly. Why is the callback ugly? I thought it was a great way to extend pretty-formats, without teaching pretty.c about every possible format that callers could ever want. There's also the problem with sorting and quoting (both only work with for-each-ref atoms only). Why would I want to sort by reflog-identity-name (or something) in for-each-ref? The sensible fields for sorting in for-each-ref are all for-each-ref atoms. On quoting, I agree. We must move the quoting to pretty.c eventually, but I don't think it is urgent. A better solution may be improving pretty.c to the point where it can more or less replace f-e-r's --format. Why would you want to stuff everything into pretty.c? If any callers wants to implement one specialized format, the only way to do it is to stuff it into the One True pretty-formats? Even more, I think pretty engine should be easily added to cat-file (especially --batch), as a generic way to extract information. Cute theoretical exercise. As usual, I'm not interested: this topic is about making git-branch more awesome, not playing with pretty-formats. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-daemon: needs /root/.config/git/config?
On Sun, Jun 09, 2013 at 02:47:57PM +0200, Bernhard R. Link wrote: * Ian Kumlien po...@vapor.com [130605 13:31]: Yes, i agree, it's suboptimal but I for one would use getpwuid to get the home directory of the executing user to avoid this - though i don't know how portable it is (or if there is any other issues) It's not only suboptimal but simply wrong. getpwuid gives at best the initial home directory, and even there it is only a guess. (If you are looking for some home directory of a different user it might be a good guess). But using getpwuid(getuid())-pw_dir if HOME is set is a serious mistake, as you throw out the good value for some almost but not quite totally unrelated value. Well i never intended for it to replace the environment variable, it was more intended as a fallback - if there will be a less strict mode then perhaps a fallback would be a more controled way of doing it. Bernhard R. Link -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] rm: better error message on failure for multiple files
When 'git rm' fails, it now displays a single message
with the list of files involved, instead of displaying
a list of messages with one file each.
As an example, the old message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
error: 'bar.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would now be displayed as:
error: the following files have changes staged in the index:
foo.txt
bar.txt
(use --cached to keep the file, or -f to force removal)
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
builtin/rm.c | 93 +-
1 files changed, 79 insertions(+), 14 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..1bff656 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -82,6 +82,11 @@ static int check_local_mod(unsigned char *head, int
index_only)
int i, no_head;
int errs = 0;
+ struct string_list files_staged = STRING_LIST_INIT_NODUP;
+ struct string_list files_cached = STRING_LIST_INIT_NODUP;
+ struct string_list files_submodule = STRING_LIST_INIT_NODUP;
+ struct string_list files_local = STRING_LIST_INIT_NODUP;
+
no_head = is_null_sha1(head);
for (i = 0; i list.nr; i++) {
struct stat st;
@@ -171,29 +176,89 @@ static int check_local_mod(unsigned char *head, int
index_only)
*/
if (local_changes staged_changes) {
if (!index_only || !(ce-ce_flags CE_INTENT_TO_ADD))
- errs = error(_('%s' has staged content
different
-from both the file and the HEAD\n
-(use -f to force removal)),
name);
+ string_list_append(files_staged, name);
}
else if (!index_only) {
if (staged_changes)
- errs = error(_('%s' has changes staged in the
index\n
-(use --cached to keep the file,
-or -f to force removal)), name);
+ string_list_append(files_cached, name);
if (local_changes) {
if (S_ISGITLINK(ce-ce_mode)
!submodule_uses_gitfile(name)) {
- errs = error(_(submodule '%s' (or one
of its nested
-submodules) uses a .git
directory\n
-(use 'rm -rf' if you
really want to remove
-it including all of its
history)), name);
- } else
- errs = error(_('%s' has local
modifications\n
-(use --cached to keep the
file,
-or -f to force
removal)), name);
+ string_list_append(files_submodule,
+ name);
+ } else {
+ string_list_append(files_local, name);
+ }
}
}
}
+ if (files_staged.nr) {
+ struct strbuf msg_staged = STRBUF_INIT;
+ int j;
+ strbuf_addstr(
+ msg_staged,
+ the following files have staged content different
+ from both the\nfile and the HEAD:);
+ for (j = 0; j files_staged.nr; j++) {
+ strbuf_addf(msg_staged,
+ \n %s,
+ files_staged.items[j].string);
+ }
+ strbuf_addstr(msg_staged,
+ \n(use -f to force removal));
+ errs = error(_(%s), msg_staged.buf);
+ }
+ if (files_cached.nr) {
+ struct strbuf msg_cached = STRBUF_INIT;
+ int j;
+ strbuf_addstr(
+ msg_cached,
+ the following files have changes staged
+ in the index:);
+ for (j = 0; j files_cached.nr; j++) {
+ strbuf_addf(msg_cached,
+ \n %s,
+ files_cached.items[j].string);
+ }
+
[PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.
As an example, the message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would look like, with advice.rmHints=false:
error: 'foo.txt' has changes staged in the index
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Documentation/config.txt |3 ++
advice.c |2 +
advice.h |1 +
builtin/rm.c | 36 -
t/t3600-rm.sh| 77 ++
5 files changed, 104 insertions(+), 15 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
amWorkDir::
Advice that shows the location of the patch file when
linkgit:git-am[1] fails to apply it.
+ rmHints::
+ In case of failure in the output of linkgit:git-rm[1],
+ show directions on how to proceed from the current state.
--
core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
static struct {
const char *name;
@@ -33,6 +34,7 @@ static struct {
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
{ setupstreamfailure, advice_set_upstream_failure },
+ { rmhints, advice_rm_hints },
/* make this an alias for backward compatibility */
{ pushnonfastforward, advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 1bff656..c9081cd 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
if (!submodule_uses_gitfile(name))
errs = error(_(submodule '%s' (or one of its nested
-submodules) uses a .git directory\n
-(use 'rm -rf' if you really want to
remove
-it including all of its history)), name);
+ submodules) uses a .git directory%s),
name,
+ advice_rm_hints
+ ? \n(use 'rm -rf' if you really want to
remove
+ it including all of its history)
+ : );
}
return errs;
@@ -204,8 +206,9 @@ static int check_local_mod(unsigned char *head, int
index_only)
\n %s,
files_staged.items[j].string);
}
- strbuf_addstr(msg_staged,
- \n(use -f to force removal));
+ if (advice_rm_hints)
+ strbuf_addstr(msg_staged,
+ \n(use -f to force removal));
errs = error(_(%s), msg_staged.buf);
}
if (files_cached.nr) {
@@ -220,9 +223,10 @@ static int check_local_mod(unsigned char *head, int
index_only)
\n %s,
files_cached.items[j].string);
}
- strbuf_addstr(msg_cached,
- \n(use --cached to keep the file,
- or -f to force removal));
+ if (advice_rm_hints)
+ strbuf_addstr(msg_cached,
+ \n(use --cached to keep the file,
+ or -f to force removal));
errs = error(_(%s), msg_cached.buf);
}
if (files_submodule.nr) {
@@ -237,10 +241,11 @@ static int check_local_mod(unsigned char *head, int
index_only)
\n %s,
files_submodule.items[j].string);
}
- strbuf_addstr(msg_sub,
- \n(use 'rm -rf' if you really
Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Please ignore this, manipulation error while in the git send-email
command line.
Le 2013-06-10 14:51, Mathieu Lienard--Mayor a écrit :
Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current
behavior.
As an example, the message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would look like, with advice.rmHints=false:
error: 'foo.txt' has changes staged in the index
Signed-off-by: Mathieu Lienard--Mayor
mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Documentation/config.txt |3 ++
advice.c |2 +
advice.h |1 +
builtin/rm.c | 36 -
t/t3600-rm.sh| 77
++
5 files changed, 104 insertions(+), 15 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
amWorkDir::
Advice that shows the location of the patch file when
linkgit:git-am[1] fails to apply it.
+ rmHints::
+ In case of failure in the output of linkgit:git-rm[1],
+ show directions on how to proceed from the current state.
--
core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
static struct {
const char *name;
@@ -33,6 +34,7 @@ static struct {
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
{ setupstreamfailure, advice_set_upstream_failure },
+ { rmhints, advice_rm_hints },
/* make this an alias for backward compatibility */
{ pushnonfastforward, advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 1bff656..c9081cd 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
if (!submodule_uses_gitfile(name))
errs = error(_(submodule '%s' (or one of its nested
-submodules) uses a .git directory\n
-(use 'rm -rf' if you really want to remove
-it including all of its history)), name);
+ submodules) uses a .git directory%s),
name,
+ advice_rm_hints
+ ? \n(use 'rm -rf' if you really want to
remove
+ it including all of its history)
+ : );
}
return errs;
@@ -204,8 +206,9 @@ static int check_local_mod(unsigned char *head,
int index_only)
\n%s,
files_staged.items[j].string);
}
- strbuf_addstr(msg_staged,
- \n(use -f to force removal));
+ if (advice_rm_hints)
+ strbuf_addstr(msg_staged,
+ \n(use -f to force removal));
errs = error(_(%s), msg_staged.buf);
}
if (files_cached.nr) {
@@ -220,9 +223,10 @@ static int check_local_mod(unsigned char *head,
int index_only)
\n%s,
files_cached.items[j].string);
}
- strbuf_addstr(msg_cached,
- \n(use --cached to keep the file,
- or -f to force removal));
+ if (advice_rm_hints)
+ strbuf_addstr(msg_cached,
+ \n(use --cached to keep the file,
+ or -f to force removal));
errs = error(_(%s), msg_cached.buf);
}
if (files_submodule.nr) {
@@ -237,10 +241,11 @@ static int check_local_mod(unsigned char *head,
int index_only)
\n%s,
Re: [PATCH 1/2] rm: better error message on failure for multiple files
Please ignore this, manipulation error while in the git send-email
command line.
Le 2013-06-10 14:51, Mathieu Lienard--Mayor a écrit :
When 'git rm' fails, it now displays a single message
with the list of files involved, instead of displaying
a list of messages with one file each.
As an example, the old message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
error: 'bar.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would now be displayed as:
error: the following files have changes staged in the index:
foo.txt
bar.txt
(use --cached to keep the file, or -f to force removal)
Signed-off-by: Mathieu Lienard--Mayor
mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
builtin/rm.c | 93
+-
1 files changed, 79 insertions(+), 14 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..1bff656 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -82,6 +82,11 @@ static int check_local_mod(unsigned char *head,
int index_only)
int i, no_head;
int errs = 0;
+ struct string_list files_staged = STRING_LIST_INIT_NODUP;
+ struct string_list files_cached = STRING_LIST_INIT_NODUP;
+ struct string_list files_submodule = STRING_LIST_INIT_NODUP;
+ struct string_list files_local = STRING_LIST_INIT_NODUP;
+
no_head = is_null_sha1(head);
for (i = 0; i list.nr; i++) {
struct stat st;
@@ -171,29 +176,89 @@ static int check_local_mod(unsigned char *head,
int index_only)
*/
if (local_changes staged_changes) {
if (!index_only || !(ce-ce_flags CE_INTENT_TO_ADD))
- errs = error(_('%s' has staged content different
-from both the file and the HEAD\n
-(use -f to force removal)),
name);
+ string_list_append(files_staged, name);
}
else if (!index_only) {
if (staged_changes)
- errs = error(_('%s' has changes staged in the
index\n
-(use --cached to keep the file,
-or -f to force removal)), name);
+ string_list_append(files_cached, name);
if (local_changes) {
if (S_ISGITLINK(ce-ce_mode)
!submodule_uses_gitfile(name)) {
- errs = error(_(submodule '%s' (or one of
its nested
-submodules) uses a .git
directory\n
-(use 'rm -rf' if you really
want to remove
-it including all of its
history)), name);
- } else
- errs = error(_('%s' has local
modifications\n
-(use --cached to keep the
file,
-or -f to force
removal)), name);
+ string_list_append(files_submodule,
+ name);
+ } else {
+ string_list_append(files_local, name);
+ }
}
}
}
+ if (files_staged.nr) {
+ struct strbuf msg_staged = STRBUF_INIT;
+ int j;
+ strbuf_addstr(
+ msg_staged,
+ the following files have staged content different
+ from both the\nfile and the HEAD:);
+ for (j = 0; j files_staged.nr; j++) {
+ strbuf_addf(msg_staged,
+ \n%s,
+ files_staged.items[j].string);
+ }
+ strbuf_addstr(msg_staged,
+ \n(use -f to force removal));
+ errs = error(_(%s), msg_staged.buf);
+ }
+ if (files_cached.nr) {
+ struct strbuf msg_cached = STRBUF_INIT;
+ int j;
+ strbuf_addstr(
+ msg_cached,
+ the following files have changes staged
+ in the index:);
+ for (j = 0; j files_cached.nr; j++) {
+
Re: [PATCH] Documentation/CommunityGuidelines
Le 10/06/2013 15:28, Ramkumar Ramachandra a écrit : 0. You do not take offense, no matter what. If someone attacks you irrationally, you do not respond. This is a public mailing list, and we are all rational people: the attacker has already humiliated herself in public, and everyone can see that. Herself? Typo I guess :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/CommunityGuidelines
Célestin Matte celestin.ma...@ensimag.fr writes: Le 10/06/2013 15:28, Ramkumar Ramachandra a écrit : 0. You do not take offense, no matter what. If someone attacks you irrationally, you do not respond. This is a public mailing list, and we are all rational people: the attacker has already humiliated herself in public, and everyone can see that. Herself? Typo I guess :) Not necessarily. It's quite common in english to use she when the gender is not known. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.
As an example, the message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would look like, with advice.rmHints=false:
error: 'foo.txt' has changes staged in the index
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v1:
-corrected the commit message where rmHints=true was supposed to be false
-better use of English tenses in the commit message
Documentation/config.txt |3 +++
advice.c |2 ++
advice.h |1 +
builtin/rm.c | 11 +++
t/t3600-rm.sh| 29 +
5 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
amWorkDir::
Advice that shows the location of the patch file when
linkgit:git-am[1] fails to apply it.
+ rmHints::
+ In case of failure in the output of linkgit:git-rm[1],
+ show directions on how to proceed from the current state.
--
core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
static struct {
const char *name;
@@ -33,6 +34,7 @@ static struct {
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
{ setupstreamfailure, advice_set_upstream_failure },
+ { rmhints, advice_rm_hints },
/* make this an alias for backward compatibility */
{ pushnonfastforward, advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 76dfc5b..2226037 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
if (!submodule_uses_gitfile(name))
errs = error(_(submodule '%s' (or one of its nested
-submodules) uses a .git directory\n
-(use 'rm -rf' if you really want to
remove
-it including all of its history)), name);
+ submodules) uses a .git directory%s),
name,
+ advice_rm_hints
+ ? \n(use 'rm -rf' if you really want to
remove
+ it including all of its history)
+ : );
}
return errs;
@@ -85,7 +87,8 @@ static int print_error_files(struct string_list *files_list,
strbuf_addf(err_msg,
\n%s,
files_list-items[i].string);
- strbuf_addstr(err_msg, hints_msg);
+ if (advice_rm_hints)
+ strbuf_addstr(err_msg, hints_msg);
errs = error(%s, err_msg.buf);
return errs;
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 08bc9bb..92f6146 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -707,6 +707,18 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rm files with different staged content without hints' '
+ cat expect EOF
+error: the following files have staged content different from both the
+file and the HEAD:
+bar.txt
+foo.txt
+EOF
+ echo content2 foo.txt
+ echo content2 bar.txt
+ test_must_fail git -c advice.rmhints=false rm foo.txt bar.txt 2actual
+ test_cmp expect actual
+'
test_expect_success 'rm file with local modification' '
cat expect EOF
@@ -720,6 +732,15 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rm file with local modification without hints' '
+ cat expect EOF
+error: the following files have local modifications:
+bar.txt
+EOF
+ echo content4 bar.txt
+ test_must_fail git -c advice.rmhints=false rm bar.txt 2actual
+ test_cmp expect actual
+'
test_expect_success 'rm file with
[PATCH 2/2] make color.ui default to 'auto'
Most users seem to like having colors enabled, and colors can help beginners to understand the output of some commands (e.g. notice immediately the boundary between commits in the output of git log). Many tutorials tell the users to set color.ui=auto as a very first step, which tend to indicate that color.ui=none is not the recommanded value, hence should not be the default. These tutorials would benefit from skipping this step and starting the real Git manipulations earlier. Other beginners do not know about color.ui=auto, and may not discover it by themselves, hence live with blackwhite outputs while they may have preferred colors. A few people (e.g. color-blind) prefer having no colors, but they can easily set color.ui=never for this (and googling disable colors in git already tells them how to do so), but this needs not occupy space in beginner-oriented documentations. A transition period with Git emitting a warning when color.ui is unset would be possible, but the discomfort of having the warning seems superior to the benefit: users may be surprised by the change, but not harmed by it. The default value is changed, and the documentation is reworded to mention color.ui=false first, since the primary use of color.ui after this change is to disable colors, not to enable it. Signed-off-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com --- I just changed Git 2.0 with Git 1.8.4. I think a default change deserves a mention of the version in which it changed (I think it's common for someone to use an old Git version and to read a more recent documentation online, so this may avoid bad surprises). I'm fine with dropping since Git 1.8.4 completely if people think it's better. Documentation/config.txt | 11 ++- builtin/config.c | 2 +- color.c | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index e97facc..6570aee 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -905,11 +905,12 @@ color.ui:: as `color.diff` and `color.grep` that control the use of color per command family. Its scope will expand as more commands learn configuration to set a default for the `--color` option. Set it - to `always` if you want all output not intended for machine - consumption to use color, to `true` or `auto` if you want such - output to use color when written to the terminal, or to `false` or - `never` if you prefer Git commands not to use color unless enabled - explicitly with some other configuration or the `--color` option. + to `false` or `never` if you prefer Git commands not to use + color unless enabled explicitly with some other configuration + or the `--color` option. Set it to `always` if you want all + output not intended for machine consumption to use color, to + `true` or `auto` (this is the default since Git 1.8.4) if you + want such output to use color when written to the terminal. column.ui:: Specify whether supported commands should output in columns. diff --git a/builtin/config.c b/builtin/config.c index 057bb61..753449f 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -342,7 +342,7 @@ static int get_colorbool(int print) if (get_colorbool_found 0) /* default value if none found in config */ - get_colorbool_found = 0; + get_colorbool_found = GIT_COLOR_AUTO; get_colorbool_found = want_color(get_colorbool_found); diff --git a/color.c b/color.c index e8e2681..f672885 100644 --- a/color.c +++ b/color.c @@ -1,7 +1,7 @@ #include cache.h #include color.h -static int git_use_color_default = 0; +static int git_use_color_default = GIT_COLOR_AUTO; int color_stdout_is_tty = -1; /* -- 1.8.3.rc3.8.g5e49f30 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] config: refactor management of color.ui's default value
The meaning of get_colorbool_found and get_diff_color_found is the
config value if found, and -1 otherwise, but get_color_ui_found had a
slightly different meaning, as it has the value 0 (which corresponds to
the default value from the user point of view) when color.ui is unset.
Make get_color_ui_found default to -1, and make it explicit that 0 is the
default value when nothing else is found.
Signed-off-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Junio C Hamano gits...@pobox.com
---
This one is just a resend of what's already in pu.
builtin/config.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/builtin/config.c b/builtin/config.c
index 33c9bf9..057bb61 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -329,6 +329,7 @@ static int get_colorbool(int print)
{
get_colorbool_found = -1;
get_diff_color_found = -1;
+ get_color_ui_found = -1;
git_config_with_options(git_get_colorbool_config, NULL,
given_config_file, respect_includes);
@@ -339,6 +340,10 @@ static int get_colorbool(int print)
get_colorbool_found = get_color_ui_found;
}
+ if (get_colorbool_found 0)
+ /* default value if none found in config */
+ get_colorbool_found = 0;
+
get_colorbool_found = want_color(get_colorbool_found);
if (print) {
--
1.8.3.rc3.8.g5e49f30
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] rm: better error message on failure for multiple files
When 'git rm' fails, it now displays a single message
with the list of files involved, instead of displaying
a list of messages with one file each.
As an example, the old message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
error: 'bar.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would now be displayed as:
error: the following files have changes staged in the index:
foo.txt
bar.txt
(use --cached to keep the file, or -f to force removal)
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v1:
-introduction of print_error_files() to avoid repetitions
-implementation with string_list instead of strbuf
-typo fileand switched to file and
builtin/rm.c | 74 ++--
t/t3600-rm.sh | 48 +
2 files changed, 108 insertions(+), 14 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..76dfc5b 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -70,6 +70,27 @@ static int check_submodules_use_gitfiles(void)
return errs;
}
+/*
+ * PRECONDITION: files_list is a non-empty string_list
+ */
+static int print_error_files(struct string_list *files_list,
+const char *main_msg,
+const char *hints_msg)
+{
+ int errs = 0;
+ struct strbuf err_msg = STRBUF_INIT;
+ int i;
+ strbuf_addstr(err_msg, main_msg);
+ for (i = 0; i files_list-nr; i++)
+ strbuf_addf(err_msg,
+ \n%s,
+ files_list-items[i].string);
+ strbuf_addstr(err_msg, hints_msg);
+ errs = error(%s, err_msg.buf);
+
+ return errs;
+}
+
static int check_local_mod(unsigned char *head, int index_only)
{
/*
@@ -82,6 +103,11 @@ static int check_local_mod(unsigned char *head, int
index_only)
int i, no_head;
int errs = 0;
+ struct string_list files_staged = STRING_LIST_INIT_NODUP;
+ struct string_list files_cached = STRING_LIST_INIT_NODUP;
+ struct string_list files_submodule = STRING_LIST_INIT_NODUP;
+ struct string_list files_local = STRING_LIST_INIT_NODUP;
+
no_head = is_null_sha1(head);
for (i = 0; i list.nr; i++) {
struct stat st;
@@ -171,29 +197,49 @@ static int check_local_mod(unsigned char *head, int
index_only)
*/
if (local_changes staged_changes) {
if (!index_only || !(ce-ce_flags CE_INTENT_TO_ADD))
- errs = error(_('%s' has staged content
different
-from both the file and the HEAD\n
-(use -f to force removal)),
name);
+ string_list_append(files_staged, name);
}
else if (!index_only) {
if (staged_changes)
- errs = error(_('%s' has changes staged in the
index\n
-(use --cached to keep the file,
-or -f to force removal)), name);
+ string_list_append(files_cached, name);
if (local_changes) {
if (S_ISGITLINK(ce-ce_mode)
!submodule_uses_gitfile(name)) {
- errs = error(_(submodule '%s' (or one
of its nested
-submodules) uses a .git
directory\n
-(use 'rm -rf' if you
really want to remove
-it including all of its
history)), name);
- } else
- errs = error(_('%s' has local
modifications\n
-(use --cached to keep the
file,
-or -f to force
removal)), name);
+ string_list_append(files_submodule,
+ name);
+ } else {
+ string_list_append(files_local, name);
+ }
}
}
}
+ if (files_staged.nr)
+ errs = print_error_files(files_staged,
+_(the following files have staged
+
[PATCH v2 1/2] status: introduce status.short to enable --short by default
Some people always run 'git status -s'.
The configuration variable status.short allows to set it by default.
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v1:
-documentation more accurate
-removal of unappropriate function calls
-clean-up in the test
Documentation/config.txt |4
builtin/commit.c |5 +
t/t7508-status.sh| 35 +++
3 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..1983bf7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2066,6 +2066,10 @@ status.relativePaths::
relative to the repository root (this was the default for Git
prior to v1.5.4).
+status.short::
+ Set to true to enable --short by default in linkgit:git-status[1].
+ The option --no-short takes precedence over this variable.
+
status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 1621dfc..287f1cb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1112,6 +1112,11 @@ static int git_status_config(const char *k, const char
*v, void *cb)
s-submodule_summary = -1;
return 0;
}
+ if (!strcmp(k, status.short)) {
+ if (git_config_bool(k, v))
+ status_format = STATUS_FORMAT_SHORT;
+ return 0;
+ }
if (!strcmp(k, status.color) || !strcmp(k, color.status)) {
s-use_color = git_config_colorbool(k, v);
return 0;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e2ffdac..9a07f15 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1335,4 +1335,39 @@ test_expect_failure '.git/config ignore=all suppresses
submodule summary' '
git config -f .gitmodules --remove-section submodule.subname
'
+test_expect_success 'Setup of environment of test' '
+ git config status.showUntrackedFiles no
+'
+
+test_expect_success 'status.short=true same as -s' '
+ git -c status.short=true status actual
+ git status -s expected_short
+ test_cmp actual expected_short
+'
+
+test_expect_success 'status.short=true different from --no-short' '
+ git status --no-short expected_noshort
+ test_must_fail test_cmp actual expected_noshort
+'
+
+test_expect_success 'status.short=true weaker than --no-short' '
+ git -c status.short=true status --no-short actual
+ test_cmp actual expected_noshort
+'
+
+test_expect_success 'status.short=false same as --no-short' '
+ git -c status.short=false status actual
+ git status -s expected_short
+ test_cmp actual expected_noshort
+'
+
+test_expect_success 'status.short=false weaker than -s' '
+ git -c status.short=false status -s actual
+ test_cmp actual expected_short
+'
+
+test_expect_success 'Back to environment of test by default' '
+ git config status.showUntrackedFiles yes
+'
+
test_done
--
1.7.8
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git diff bug?
On Mon, Jun 10, 2013 at 8:44 AM, Célestin Matte celestin.ma...@ensimag.fr wrote: Since nobody answered you (publicly at least), I will try doing it myself: I think the best thing to do if you want a feature to be added is to come with a patch and request for comments on it. Then, people will discuss it and decide whether it's worth adding it to git. So yes, you can try implementing it - all work is welcome :) That sounds great. I will try implementing and send out a patch soon! -- 010 001 111 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] status: introduce status.short to enable --short by default
Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr writes: +test_expect_success 'Setup of environment of test' ' Why these double quotes inside single quotes? +test_expect_success 'Back to environment of test by default' ' Same. test environment would sound better than environment of test in english. + git config status.showUntrackedFiles yes yes is not documented as an acceptable value for status.showUntrackedFiles. git config --unset is what you want here. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
gitk error on checkout of branch
Hi, using gitk (git 1.8.3), the program gets stuck when checking out a new branch if the last checked-out branch was the bottom left branch. The error occurs consistently in this repository, but unfortunately, I cannot reproduce the error on another repository. The message reads: invalid command name .bleft.bottom.ctext while executing $ctext conf -state normal (procedure getblobdiffline line 10) invoked from within getblobdiffline file6 5f7f5bd07fb3de12ce64dee4f19e060c9d62cc54 (eval body line 1) invoked from within eval $script (procedure dorunq line 11) invoked from within dorunq (after script) any suggestions on what to do are appreciated. kind regards, Steven -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] status:introduce status.branch to enable --branch by default
Some people often run 'git status -b'.
The config variable status.branch allows to set it by default.
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v1:
-Documentation more accurate
-removal of unappropriate function calls
-clean up in the test
Documentation/config.txt |4
builtin/commit.c |4
t/t7508-status.sh| 27 +++
3 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1983bf7..ecdcd6d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2070,6 +2070,10 @@ status.short::
Set to true to enable --short by default in linkgit:git-status[1].
The option --no-short takes precedence over this variable.
+status.branch::
+ Set to true to enable --branch by default in linkgit:git-status[1].
+ The option --no-branch takes precedence over this variable.
+
status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 287f1cb..f2b5d44 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1117,6 +1117,10 @@ static int git_status_config(const char *k, const char
*v, void *cb)
status_format = STATUS_FORMAT_SHORT;
return 0;
}
+ if (!strcmp(k, status.branch)) {
+ s-show_branch = git_config_bool(k, v);
+ return 0;
+ }
if (!strcmp(k, status.color) || !strcmp(k, color.status)) {
s-use_color = git_config_colorbool(k, v);
return 0;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 9a07f15..958617a 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1366,6 +1366,33 @@ test_expect_success 'status.short=false weaker than
-s' '
test_cmp actual expected_short
'
+test_expect_success 'status.branch=true same as -b' '
+ git -c status.branch=true status -s actual
+ git status -sb expected_branch
+ test_cmp actual expected_branch
+'
+
+test_expect_success 'status.branch=true different from --no-branch' '
+ git -c status.branch=true status -s actual
+ git status -s --no-branch expected_nobranch
+ test_must_fail test_cmp actual expected_nobranch
+'
+
+test_expect_success 'status.branch=true weaker than --no-branch' '
+ git -c status.branch=true status -s --no-branch actual
+ test_cmp actual expected_nobranch
+'
+
+test_expect_success 'status.branch=false same as --no-branch' '
+ git -c status.branch=false status -s actual
+ test_cmp actual expected_nobranch
+'
+
+test_expect_success 'status.branch=false weaker than -b' '
+ git -c status.branch=false status -sb actual
+ test_cmp actual expected_branch
+'
+
test_expect_success 'Back to environment of test by default' '
git config status.showUntrackedFiles yes
'
--
1.7.8
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] rm: better error message on failure for multiple files
Le 2013-06-10 16:38, Matthieu Moy a écrit : Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr writes: When 'git rm' fails, it now displays a single message with the list of files involved, instead of displaying a list of messages with one file each. As an example, the old message: error: 'foo.txt' has changes staged in the index (use --cached to keep the file, or -f to force removal) error: 'bar.txt' has changes staged in the index (use --cached to keep the file, or -f to force removal) would now be displayed as: error: the following files have changes staged in the index: foo.txt bar.txt (use --cached to keep the file, or -f to force removal) Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr Signed-off-by: Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr Signed-off-by: Matthieu Moy matthieu@grenoble-inp.frlist There's a list after my email, probably a typo. yes, that's a leftover from a rebase-i +/* + * PRECONDITION: files_list is a non-empty string_list + */ Avoid repeating in comments what the code already says. file_list is non-empty is sufficient, we already know it's a string_list. Okay + if (files_staged.nr) + errs = print_error_files(files_staged, +_(the following files have staged + content different from both the + \nfile and the HEAD:), +_(\n(use -f to force removal))); + if (files_cached.nr) + errs = print_error_files(files_cached, +_(the following files have changes + staged in the index:), +_(\n(use --cached to keep the file, + or -f to force removal))); What happens if both conditions are true? It seems the second will override the first. I think it'd be OK because what matters is that errs is set by someone, no matter who, and the error message is displayed on screen, not contained in the variable, but this looks weird. I'd find it more readable with errs |= print_error_files(...). Well the current code is only using errs=error(...), using the same variable errs over and over, no matter how many times it loops. That's why i implemented it similarly. And actually, you may want to move the if (nr) inside print_error_files (wich could then be called print_error_files_maybe). At least, there should be a test where two conditions are true. I'll do that, to be sure about the behaviour. + if (files_submodule.nr) + errs = print_error_files(files_submodule, +_(the following submodules (or one + of its nested submodule) use a + .git directory:), +_(\n(use 'rm -rf' if you really + want to remove i including all i - it ? -- Mathieu Liénard--Mayor, 2nd year at Grenoble INP - ENSIMAG (+33)6 80 56 30 02 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] rm: better error message on failure for multiple files
Mathieu Liénard--Mayor mathieu.lienard--ma...@ensimag.fr writes: Well the current code is only using errs=error(...), using the same variable errs over and over, no matter how many times it loops. That's why i implemented it similarly. OK, consistency is a good argument then. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] test: improve rebase -q test
SZEDER Gábor sze...@ira.uka.de writes: On Sun, Jun 09, 2013 at 03:41:54PM -0500, Felipe Contreras wrote: There will not be a need for test_string_must_be_empty() just like there's no need for test_string_cmp(). Actually, if there were a test_string_cmp(), that would be the test helper function I used most often. Hmm, there indeed are quite a many At this point, the variable's value must be this in the test scripts. With things like this: t/t0002-gitfile.sh: test $SHA = $(git rev-list HEAD) we can go to the trash directory upon seeing a failure to run the command used on the RHS, but the value in $SHA is cumbersome to find out (either running it under sh -x or insert an extra echo before it), so such a helper function may be useful. Do you really need a general comparison (does A sort before B) or just equality? If the latter, test_string_equal (or even string_equal) might be a better name for it. By the way, test_cmp() is a replacement for the cmp command and that is why it does not have file in its name. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] rm: introduce advice.rmHints to shorten messages
Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.
As an example, the message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would look like, with advice.rmHints=false:
error: 'foo.txt' has changes staged in the index
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Documentation/config.txt |3 +++
advice.c |2 ++
advice.h |1 +
builtin/rm.c | 11 +++
t/t3600-rm.sh| 29 +
5 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
amWorkDir::
Advice that shows the location of the patch file when
linkgit:git-am[1] fails to apply it.
+ rmHints::
+ In case of failure in the output of linkgit:git-rm[1],
+ show directions on how to proceed from the current state.
--
core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
static struct {
const char *name;
@@ -33,6 +34,7 @@ static struct {
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
{ setupstreamfailure, advice_set_upstream_failure },
+ { rmhints, advice_rm_hints },
/* make this an alias for backward compatibility */
{ pushnonfastforward, advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 07306eb..c991fe6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
if (!submodule_uses_gitfile(name))
errs = error(_(submodule '%s' (or one of its nested
-submodules) uses a .git directory\n
-(use 'rm -rf' if you really want to
remove
-it including all of its history)), name);
+ submodules) uses a .git directory%s),
name,
+ advice_rm_hints
+ ? \n(use 'rm -rf' if you really want to
remove
+ it including all of its history)
+ : );
}
return errs;
@@ -83,7 +85,8 @@ static void print_eventual_error_files(struct string_list
*files_list,
strbuf_addf(err_msg,
\n%s,
files_list-items[i].string);
- strbuf_addstr(err_msg, hints_msg);
+ if (advice_rm_hints)
+ strbuf_addstr(err_msg, hints_msg);
*errs = error(%s, err_msg.buf);
}
}
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 10dd380..74f048c 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -707,6 +707,18 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rm files with different staged content without hints' '
+ cat expect EOF
+error: the following files have staged content different from both the
+file and the HEAD:
+bar.txt
+foo.txt
+EOF
+ echo content2 foo.txt
+ echo content2 bar.txt
+ test_must_fail git -c advice.rmhints=false rm foo.txt bar.txt 2actual
+ test_cmp expect actual
+'
test_expect_success 'rm file with local modification' '
cat expect EOF
@@ -720,6 +732,15 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rm file with local modification without hints' '
+ cat expect EOF
+error: the following files have local modifications:
+bar.txt
+EOF
+ echo content4 bar.txt
+ test_must_fail git -c advice.rmhints=false rm bar.txt 2actual
+ test_cmp expect actual
+'
test_expect_success 'rm file with changes in the index' '
cat expect EOF
@@ -734,6 +755,14 @@ EOF
test_cmp
Re: [PATCH v2 00/15] Towards a more awesome git branch
Ramkumar Ramachandra artag...@gmail.com writes: [-CC: Duy, since he has left the community] Junio: since Duy is no longer around to guide us, I will rely on your guidance. Duy Nguyen wrote: I'm starting to think this is a half-baked solution. It hides problems, for example commit placeholders should produce empty string, not the literal placeholders. Why should they produce empty strings? Aren't they equivalent to invalid placeholders? I haven't even skimmed the series, so responding to a comment given as a response to 00/15 is a bit difficult, but I _think_ this refers to this issue: what should %(cD) produce when given to for-each-ref and you have a tag that points at non committish? http://thread.gmane.org/gmane.comp.version-control.git/226343/focus=226460 I may be guessing incorrectly what Duy's comment is about, though. I'll hopefully have something more concrete and useful after I have a chance to read the series, but not now. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] rm: better error message on failure for multiple files
When 'git rm' fails, it now displays a single message
with the list of files involved, instead of displaying
a list of messages with one file each.
As an example, the old message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
error: 'bar.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would now be displayed as:
error: the following files have changes staged in the index:
foo.txt
bar.txt
(use --cached to keep the file, or -f to force removal)
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v2:
-couple typo in commit message and in code
-rename and redefinition of the intermediate function
-move the 4 if(nr) inside the function
builtin/rm.c | 71 +---
t/t3600-rm.sh | 67 +
2 files changed, 124 insertions(+), 14 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..07306eb 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -70,6 +70,24 @@ static int check_submodules_use_gitfiles(void)
return errs;
}
+static void print_eventual_error_files(struct string_list *files_list,
+ const char *main_msg,
+ const char *hints_msg,
+ int *errs)
+{
+ if (files_list-nr) {
+ struct strbuf err_msg = STRBUF_INIT;
+ int i;
+ strbuf_addstr(err_msg, main_msg);
+ for (i = 0; i files_list-nr; i++)
+ strbuf_addf(err_msg,
+ \n%s,
+ files_list-items[i].string);
+ strbuf_addstr(err_msg, hints_msg);
+ *errs = error(%s, err_msg.buf);
+ }
+}
+
static int check_local_mod(unsigned char *head, int index_only)
{
/*
@@ -82,6 +100,11 @@ static int check_local_mod(unsigned char *head, int
index_only)
int i, no_head;
int errs = 0;
+ struct string_list files_staged = STRING_LIST_INIT_NODUP;
+ struct string_list files_cached = STRING_LIST_INIT_NODUP;
+ struct string_list files_submodule = STRING_LIST_INIT_NODUP;
+ struct string_list files_local = STRING_LIST_INIT_NODUP;
+
no_head = is_null_sha1(head);
for (i = 0; i list.nr; i++) {
struct stat st;
@@ -171,29 +194,49 @@ static int check_local_mod(unsigned char *head, int
index_only)
*/
if (local_changes staged_changes) {
if (!index_only || !(ce-ce_flags CE_INTENT_TO_ADD))
- errs = error(_('%s' has staged content
different
-from both the file and the HEAD\n
-(use -f to force removal)),
name);
+ string_list_append(files_staged, name);
}
else if (!index_only) {
if (staged_changes)
- errs = error(_('%s' has changes staged in the
index\n
-(use --cached to keep the file,
-or -f to force removal)), name);
+ string_list_append(files_cached, name);
if (local_changes) {
if (S_ISGITLINK(ce-ce_mode)
!submodule_uses_gitfile(name)) {
- errs = error(_(submodule '%s' (or one
of its nested
-submodules) uses a .git
directory\n
-(use 'rm -rf' if you
really want to remove
-it including all of its
history)), name);
- } else
- errs = error(_('%s' has local
modifications\n
-(use --cached to keep the
file,
-or -f to force
removal)), name);
+ string_list_append(files_submodule,
+ name);
+ } else {
+ string_list_append(files_local, name);
+ }
}
}
}
+ print_eventual_error_files(files_staged,
+ _(the
[PATCH v3 1/2] rm: better error message on failure for multiple files
When 'git rm' fails, it now displays a single message
with the list of files involved, instead of displaying
a list of messages with one file each.
As an example, the old message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
error: 'bar.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would now be displayed as:
error: the following files have changes staged in the index:
foo.txt
bar.txt
(use --cached to keep the file, or -f to force removal)
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v2:
-couple typo in commit message and in code
-rename and redefinition of the intermediate function
-move if(nr) inside the function
builtin/rm.c | 71 +---
t/t3600-rm.sh | 67 +
2 files changed, 124 insertions(+), 14 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..07306eb 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -70,6 +70,24 @@ static int check_submodules_use_gitfiles(void)
return errs;
}
+static void print_eventual_error_files(struct string_list *files_list,
+ const char *main_msg,
+ const char *hints_msg,
+ int *errs)
+{
+ if (files_list-nr) {
+ struct strbuf err_msg = STRBUF_INIT;
+ int i;
+ strbuf_addstr(err_msg, main_msg);
+ for (i = 0; i files_list-nr; i++)
+ strbuf_addf(err_msg,
+ \n%s,
+ files_list-items[i].string);
+ strbuf_addstr(err_msg, hints_msg);
+ *errs = error(%s, err_msg.buf);
+ }
+}
+
static int check_local_mod(unsigned char *head, int index_only)
{
/*
@@ -82,6 +100,11 @@ static int check_local_mod(unsigned char *head, int
index_only)
int i, no_head;
int errs = 0;
+ struct string_list files_staged = STRING_LIST_INIT_NODUP;
+ struct string_list files_cached = STRING_LIST_INIT_NODUP;
+ struct string_list files_submodule = STRING_LIST_INIT_NODUP;
+ struct string_list files_local = STRING_LIST_INIT_NODUP;
+
no_head = is_null_sha1(head);
for (i = 0; i list.nr; i++) {
struct stat st;
@@ -171,29 +194,49 @@ static int check_local_mod(unsigned char *head, int
index_only)
*/
if (local_changes staged_changes) {
if (!index_only || !(ce-ce_flags CE_INTENT_TO_ADD))
- errs = error(_('%s' has staged content
different
-from both the file and the HEAD\n
-(use -f to force removal)),
name);
+ string_list_append(files_staged, name);
}
else if (!index_only) {
if (staged_changes)
- errs = error(_('%s' has changes staged in the
index\n
-(use --cached to keep the file,
-or -f to force removal)), name);
+ string_list_append(files_cached, name);
if (local_changes) {
if (S_ISGITLINK(ce-ce_mode)
!submodule_uses_gitfile(name)) {
- errs = error(_(submodule '%s' (or one
of its nested
-submodules) uses a .git
directory\n
-(use 'rm -rf' if you
really want to remove
-it including all of its
history)), name);
- } else
- errs = error(_('%s' has local
modifications\n
-(use --cached to keep the
file,
-or -f to force
removal)), name);
+ string_list_append(files_submodule,
+ name);
+ } else {
+ string_list_append(files_local, name);
+ }
}
}
}
+ print_eventual_error_files(files_staged,
+ _(the following
Re: [PATCH v3 1/2] rm: better error message on failure for multiple files
Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr writes: +static void print_eventual_error_files(struct string_list *files_list, Too french ;-). Eventual (en) = final, utlime (fr). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] rm: introduce advice.rmHints to shorten messages
Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.
As an example, the message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would look like, with advice.rmHints=false:
error: 'foo.txt' has changes staged in the index
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Documentation/config.txt |3 +++
advice.c |2 ++
advice.h |1 +
builtin/rm.c | 11 +++
t/t3600-rm.sh| 29 +
5 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
amWorkDir::
Advice that shows the location of the patch file when
linkgit:git-am[1] fails to apply it.
+ rmHints::
+ In case of failure in the output of linkgit:git-rm[1],
+ show directions on how to proceed from the current state.
--
core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
static struct {
const char *name;
@@ -33,6 +34,7 @@ static struct {
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
{ setupstreamfailure, advice_set_upstream_failure },
+ { rmhints, advice_rm_hints },
/* make this an alias for backward compatibility */
{ pushnonfastforward, advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 07306eb..c991fe6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
if (!submodule_uses_gitfile(name))
errs = error(_(submodule '%s' (or one of its nested
-submodules) uses a .git directory\n
-(use 'rm -rf' if you really want to
remove
-it including all of its history)), name);
+ submodules) uses a .git directory%s),
name,
+ advice_rm_hints
+ ? \n(use 'rm -rf' if you really want to
remove
+ it including all of its history)
+ : );
}
return errs;
@@ -83,7 +85,8 @@ static void print_eventual_error_files(struct string_list
*files_list,
strbuf_addf(err_msg,
\n%s,
files_list-items[i].string);
- strbuf_addstr(err_msg, hints_msg);
+ if (advice_rm_hints)
+ strbuf_addstr(err_msg, hints_msg);
*errs = error(%s, err_msg.buf);
}
}
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 10dd380..74f048c 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -707,6 +707,18 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rm files with different staged content without hints' '
+ cat expect EOF
+error: the following files have staged content different from both the
+file and the HEAD:
+bar.txt
+foo.txt
+EOF
+ echo content2 foo.txt
+ echo content2 bar.txt
+ test_must_fail git -c advice.rmhints=false rm foo.txt bar.txt 2actual
+ test_cmp expect actual
+'
test_expect_success 'rm file with local modification' '
cat expect EOF
@@ -720,6 +732,15 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rm file with local modification without hints' '
+ cat expect EOF
+error: the following files have local modifications:
+bar.txt
+EOF
+ echo content4 bar.txt
+ test_must_fail git -c advice.rmhints=false rm bar.txt 2actual
+ test_cmp expect actual
+'
test_expect_success 'rm file with changes in the index' '
cat expect EOF
@@ -734,6 +755,14 @@ EOF
test_cmp
Re: [PATCH 2/4] push: make upstream, simple work with pushdefault
Junio C Hamano wrote: Only if I want to publish the result of the work forked from your triangle as my triangle, but that is not the case. A fork to be integrated by other is by definition more specialized than the original, and I would publish my pushbranch subtopic as such, not as triangle. Okay, so it should _never_ push out as triangle, because it is an insane default. But that is not what is happening, unless you keep thinking push.default decides the name of the branch regardless of what repository it lives in, which is where our difference lies, I think. I assumed that we want all push.default modes to do something sensible in both central and triangular workflows. push.default dictating a push destination would break triangular workflows. It's very clear to me. Imagine the case where I forked two branches from your triangle topic and pushing them to my repository using the triangular workflow. By your definition, they will both try to push to triangle, which means you, the triangle topic maintainer, cannot receive two independent pull requests from me. You can only get a single branch that pre-merges both, and if you want to get one but not the other, you have to do the untangling of these two topics. Okay, you've made your point. Due to namespace conflict, the solution I proposed is not at all a sane default. It's just confusing and Bad (TM). I am not saying that you have to pick one to use for push.default among the remaining ones (i.e. matching, current, what else?). It is very plausible that the triangular workflow wants a different logic to pick what branches are to be updated and how. Perhaps we would want something that is capable of mapping your local branch name to a branch name suitable in your publishing repository, and I am not opposed to have such a mode. Okay, we'll have to do some sort of split and mark push.default = upstream/ simple suitable-only-for-centralized-workflows, or something to that effect (deprecation?) :| I'll try to think of something. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Fix git checkout - (early preview)
Hi, So, this 'git checkout -' not working after a 'rebase -i' has annoyed me to no end. This is the fix. Unfortunately, some tests fail and I'm still tracking down what exactly is going on. Thanks. Ramkumar Ramachandra (3): t/checkout-last: checkout - doesn't work after rebase -i checkout: respect GIT_REFLOG_ACTION rebase -i: write better reflog messages for start builtin/checkout.c | 11 --- git-rebase--interactive.sh | 2 ++ t/t2012-checkout-last.sh | 8 3 files changed, 18 insertions(+), 3 deletions(-) -- 1.8.3.254.g60f9e5b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] t/checkout-last: checkout - doesn't work after rebase -i
The following command $ git checkout - does not work as expected after a 'git rebase -i'. Add a failing test documenting this bug. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t2012-checkout-last.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh index b44de9d..5729487 100755 --- a/t/t2012-checkout-last.sh +++ b/t/t2012-checkout-last.sh @@ -116,4 +116,12 @@ test_expect_success 'master...' ' test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify master^) ' +test_expect_failure 'checkout - works after an interactive rebase' ' + git checkout master + git checkout other + git rebase -i master + git checkout - + test z$(git symbolic-ref HEAD) = zrefs/heads/master +' + test_done -- 1.8.3.254.g60f9e5b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] rebase -i: write better reflog messages for start
Invoking 'git rebase -i' writes the following line to the reflog at the start of the operation: rebase -i (start) This is not very useful. Make it more informative like: rebase -i (start): checkout master Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- git-rebase--interactive.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 5822b2c..a05a6e4 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -837,6 +837,7 @@ comment_for_reflog start if test ! -z $switch_to then + GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $switch_to output git checkout $switch_to -- || die Could not checkout $switch_to fi @@ -980,6 +981,7 @@ has_action $todo || test -d $rewritten || test -n $force_rebase || skip_unnecessary_picks +GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $onto_name output git checkout $onto || die_abort could not detach HEAD git update-ref ORIG_HEAD $orig_head do_rest -- 1.8.3.254.g60f9e5b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] checkout: respect GIT_REFLOG_ACTION
GIT_REFLOG_ACTION is an environment variable specifying the reflog
message to write after an action is completed. Other commands including
merge, reset, and commit respect it.
This incidentally fixes a bug in t/checkout-last. You can now expect
$ git checkout -
to work fine after an interactive rebase.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
builtin/checkout.c | 11 ---
t/t2012-checkout-last.sh | 2 +-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f5b50e5..1e2af85 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -587,7 +587,7 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
struct branch_info *new)
{
struct strbuf msg = STRBUF_INIT;
- const char *old_desc;
+ const char *old_desc, *reflog_msg;
if (opts-new_branch) {
if (opts-new_orphan_branch) {
if (opts-new_branch_log !log_all_ref_updates) {
@@ -620,8 +620,13 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
old_desc = old-name;
if (!old_desc old-commit)
old_desc = sha1_to_hex(old-commit-object.sha1);
- strbuf_addf(msg, checkout: moving from %s to %s,
- old_desc ? old_desc : (invalid), new-name);
+
+ reflog_msg = getenv(GIT_REFLOG_ACTION);
+ if (!reflog_msg)
+ strbuf_addf(msg, checkout: moving from %s to %s,
+ old_desc ? old_desc : (invalid), new-name);
+ else
+ strbuf_insert(msg, 0, reflog_msg, strlen(reflog_msg));
if (!strcmp(new-name, HEAD) !new-path !opts-force_detach) {
/* Nothing to do. */
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index 5729487..ab80da7 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,7 +116,7 @@ test_expect_success 'master...' '
test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify
master^)
'
-test_expect_failure 'checkout - works after an interactive rebase' '
+test_expect_success 'checkout - works after an interactive rebase' '
git checkout master
git checkout other
git rebase -i master
--
1.8.3.254.g60f9e5b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/CommunityGuidelines
On Mon, Jun 10, 2013 at 04:04:29PM +0200, Matthieu Moy wrote: Célestin Matte celestin.ma...@ensimag.fr writes: Le 10/06/2013 15:28, Ramkumar Ramachandra a écrit : 0. You do not take offense, no matter what. If someone attacks you irrationally, you do not respond. This is a public mailing list, and we are all rational people: the attacker has already humiliated herself in public, and everyone can see that. Herself? Typo I guess :) Not necessarily. It's quite common in english to use she when the gender is not known. Could you please use themself instead? -- Robin Hugh Johnson Gentoo Linux: Developer, Trustee Infrastructure Lead E-Mail : robb...@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style
Matthieu Moy matthieu@grenoble-inp.fr writes:
Célestin Matte celestin.ma...@ensimag.fr writes:
@@ -1285,8 +1285,7 @@ sub get_mw_namespace_id {
# Look at configuration file, if the record for that namespace
is
# already cached. Namespaces are stored in form:
# Name_of_namespace:Id_namespace, ex.: File:6.
-my @temp = split(/\n/, run_git(config --get-all remote.
-. $remotename
..namespaceCache));
+my @temp = split(/\n/, run_git(config --get-all
remote.${remotename}.namespaceCache));
I tend to prefer the former, as it avoids long lines ( 80 columns)
But you split the name of a single variable across lines by doing so.
You could split lines to make it a bit more readable.
my @temp = split(/\n/,
run_git(config --get-all
remote.${remotename}.namespaceCache));
It still overflows the 80-column limit, but the namespaceCache is
the only thing that overflows; not much harm is done.
This is totally outside of the topic of coding-style series, but I
would be more concerned about the use of ${remotename}, though. Has
it (and in general the parameters to all calls to run_git()) been
sanitized for shell metacharacters? If we had a variant of run_git
that took an array of command line arguments given to git, you could
do this
my @temp = split(/\n/,
run_git([qw(config --get-all),
remote.${remotename}.namespaceCache)]);
which would be safer and easier to read.
@@ -1339,8 +1338,7 @@ sub get_mw_namespace_id {
# Store explicitely requested namespaces on disk
if (!exists $cached_mw_namespace_id{$name}) {
-run_git(config --add remote.. $remotename
-..namespaceCache \. $name .:. $store_id .\);
+run_git(qq(config --add remote.${remotename}.namespaceCache
${name}:${store_id}));
Same.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Mon, Jun 10, 2013 at 3:32 AM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Sun, Jun 9, 2013 at 4:39 PM, Junio C Hamano gits...@pobox.com wrote: One example of killing the entire thread is when I see This patch will not be applied by Felipe in a thread started with his patch. I understand that it is his way to say this patch is retracted without having to explicitly say that he now understands that reviews showed why the patch was wrong or that he thanks the reviewer for enlightening him. You are wrong. There's nothing wrong with the patch. ... I thought you understood that code should speak, but apparently you don't. That is exactly the point Peff raised (and I agreed with), isn't it? Bad behaviour (being difficult to work with) has consequences. It is not bad behavior. It is bad behavior *in your opinion*, an opinion that wouldn't be shared by other projects, like the Linux kernel. E.g. convincing people that it is not worth their time interacting with you, especially when there are better things to do like tending to other topics, and you lose the chance to show that your patches are good when they indeed are (I don't even know if these patches in question are good, and I am not going to find out). You are hurting the Git project by doing that, and our users, specially our Windows users. I thought you were a good maintainer. But apparently you would rather listen to the people that only complain, rather than actual code, that actually improves things. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
On Mon, Jun 10, 2013 at 11:53 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Jun 10, 2013 at 3:32 AM, Junio C Hamano gits...@pobox.com wrote: E.g. convincing people that it is not worth their time interacting with you, especially when there are better things to do like tending to other topics, and you lose the chance to show that your patches are good when they indeed are (I don't even know if these patches in question are good, and I am not going to find out). You are hurting the Git project by doing that, and our users, specially our Windows users. I thought you were a good maintainer. But apparently you would rather listen to the people that only complain, rather than actual code, that actually improves things. And this in fact has a name; *bias*. It is bad in any human endeavor, and in logic and argumentation, letting yourself be blinded by who is making the arguments, rather than the arguments themselves has a name; ad hominem. That is a mistake. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Mathieu Liénard--Mayor mathieu.lienard--ma...@ensimag.fr writes: Please ignore this, manipulation error while in the git send-email command line. Here is what my mailbox looks like (the penultimate one with 252 lines is what I am responding to). R. [ 146: Mathieu Lienard--Mayor ] [PATCH 1/2] rm: better error messa R. [ 231: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce advice.r R. [ 157: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce advice.r R. [ 198: Mathieu Lienard--Mayor ] [PATCH v2 1/2] rm: better error me R. [ 157: Mathieu Lienard--Mayor ] [PATCH v2 2/2] rm: introduce advic . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce advic . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2] rm: better error m . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2] rm: better error m . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce advic R [ 33: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error m O [ 38: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce advi R. [ 156: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error m R. [ 252: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce advi . [ 84: Mathieu Liénard--Mayor ] Re: [PATCH v2 1/2] rm: better erro I am guessing that [v3 1/2] and [v3 2/2] are the final ones but it that is not the case please holler. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bad attitudes and problems in the Git community (was: Re: [PATCH 2/2] Move sequencer to builtin)
On Mon, Jun 10, 2013 at 4:05 AM, Stefano Lattarini stefano.lattar...@gmail.com wrote: You need two sides to have an argument. I disagree. Unless you mean than, whenever a part behaves in a hostile and aggressive way, the other part should just silently knuckle under. You are wrong. If a bum in the street starts talking about you about why you are going to hell, and you reply to him and argue. Who has the fault of starting an argument? Both. Maybe you have even more blame. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] test: improve rebase -q test
On Mon, Jun 10, 2013 at 10:56 AM, Junio C Hamano gits...@pobox.com wrote: By the way, test_cmp() is a replacement for the cmp command and that is why it does not have file in its name. That's an irrelevant implementation detail. But if you want to be driven the the implementation, call it test_zero(). -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] rm: better error message on failure for multiple files
Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
writes:
When 'git rm' fails, it now displays a single message
with the list of files involved, instead of displaying
a list of messages with one file each.
As an example, the old message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
error: 'bar.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would now be displayed as:
error: the following files have changes staged in the index:
foo.txt
bar.txt
(use --cached to keep the file, or -f to force removal)
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v2:
-couple typo in commit message and in code
-rename and redefinition of the intermediate function
-move the 4 if(nr) inside the function
builtin/rm.c | 71 +---
t/t3600-rm.sh | 67 +
2 files changed, 124 insertions(+), 14 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 7b91d52..07306eb 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -70,6 +70,24 @@ static int check_submodules_use_gitfiles(void)
return errs;
}
+static void print_eventual_error_files(struct string_list *files_list,
+const char *main_msg,
+const char *hints_msg,
+int *errs)
Hrm, I do not see the point of eventual there, by the way. Are
there other kinds of error files?
+{
+ if (files_list-nr) {
+ struct strbuf err_msg = STRBUF_INIT;
+ int i;
+ strbuf_addstr(err_msg, main_msg);
+ for (i = 0; i files_list-nr; i++)
+ strbuf_addf(err_msg,
+ \n%s,
Is there an implication of having always 4 spaces here to l10n/i18n
here? I am wondering if it should be _(\n%s).
+ files_list-items[i].string);
+ strbuf_addstr(err_msg, hints_msg);
+ *errs = error(%s, err_msg.buf);
There needs a strbuf_release(err_msg) somewhere before leaving this
scope to avoid leaking its buffer, no?
+ }
+}
+
static int check_local_mod(unsigned char *head, int index_only)
{
/*
@@ -82,6 +100,11 @@ static int check_local_mod(unsigned char *head, int
index_only)
int i, no_head;
int errs = 0;
+ struct string_list files_staged = STRING_LIST_INIT_NODUP;
+ struct string_list files_cached = STRING_LIST_INIT_NODUP;
+ struct string_list files_submodule = STRING_LIST_INIT_NODUP;
+ struct string_list files_local = STRING_LIST_INIT_NODUP;
+
no_head = is_null_sha1(head);
for (i = 0; i list.nr; i++) {
struct stat st;
@@ -171,29 +194,49 @@ static int check_local_mod(unsigned char *head, int
index_only)
*/
if (local_changes staged_changes) {
if (!index_only || !(ce-ce_flags CE_INTENT_TO_ADD))
+ string_list_append(files_staged, name);
}
else if (!index_only) {
if (staged_changes)
- errs = error(_('%s' has changes staged in the
index\n
- (use --cached to keep the file,
- or -f to force removal)), name);
+ string_list_append(files_cached, name);
if (local_changes) {
if (S_ISGITLINK(ce-ce_mode)
!submodule_uses_gitfile(name)) {
+ string_list_append(files_submodule,
+name);
+ } else {
+ string_list_append(files_local, name);
+ }
The innermost if/else no longer needs braces. Also even though it
may push it slightly over 80-column, I think the files_submodule
side of string_list_append() is easier to read if it were on a
single line.
+ print_eventual_error_files(files_staged,
+_(the following files have staged
+ content different from both the
+ \nfile and the HEAD:),
+_(\n(use -f to force removal)),
+errs);
Hmph. I wonder if we want to properly i18n plurals, depending on
the number of files, e.g.
Re: [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style
Well, I think next step would be to replace all those calls with
Git.pm `command`, `command_oneline` and `config``which take an array
of arguments after the command. In the preview tool we use those but
I don't know if we will find the time to clean that up too in
git-remote-mediawiki :) .
Don't know though if it's better to mix that with this serie which is
mainly based on what perlcritic returns.
On 10 June 2013 18:41, Junio C Hamano gits...@pobox.com wrote:
Matthieu Moy matthieu@grenoble-inp.fr writes:
Célestin Matte celestin.ma...@ensimag.fr writes:
@@ -1285,8 +1285,7 @@ sub get_mw_namespace_id {
# Look at configuration file, if the record for that namespace
is
# already cached. Namespaces are stored in form:
# Name_of_namespace:Id_namespace, ex.: File:6.
-my @temp = split(/\n/, run_git(config --get-all remote.
-. $remotename
..namespaceCache));
+my @temp = split(/\n/, run_git(config --get-all
remote.${remotename}.namespaceCache));
I tend to prefer the former, as it avoids long lines ( 80 columns)
But you split the name of a single variable across lines by doing so.
You could split lines to make it a bit more readable.
my @temp = split(/\n/,
run_git(config --get-all
remote.${remotename}.namespaceCache));
It still overflows the 80-column limit, but the namespaceCache is
the only thing that overflows; not much harm is done.
This is totally outside of the topic of coding-style series, but I
would be more concerned about the use of ${remotename}, though. Has
it (and in general the parameters to all calls to run_git()) been
sanitized for shell metacharacters? If we had a variant of run_git
that took an array of command line arguments given to git, you could
do this
my @temp = split(/\n/,
run_git([qw(config --get-all),
remote.${remotename}.namespaceCache)]);
which would be safer and easier to read.
@@ -1339,8 +1338,7 @@ sub get_mw_namespace_id {
# Store explicitely requested namespaces on disk
if (!exists $cached_mw_namespace_id{$name}) {
-run_git(config --add remote.. $remotename
-..namespaceCache \. $name .:. $store_id .\);
+run_git(qq(config --add remote.${remotename}.namespaceCache
${name}:${store_id}));
Same.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [IGNORE] Implement 'git rebase' in ruby
On Mon, Jun 10, 2013 at 2:48 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: git-rebase.rb | 2056 + 1 file changed, 2056 insertions(+) create mode 100755 git-rebase.rb I suggest putting this in contrib/ and cooking it. As usual, my mantra is: let the patches decide what to do. I'll help review and improve this soon. What would be the point? There's only so much this script could do. I was already relying on an improved 'git cherry-pick' that's never going to happen. I think it would be more interesting to fork Git, replace all the main builtin commands with Ruby scripts that access libgit.a, and copy the code to a libgit-ng library that has stuff that is worthy of being on a library until the scripts don't access libgit.a any more. All the time passing all tests in the test framework of course. Then there would be a truly useful and stable Git library, and we would have scripts that work on all platforms, including Windows, without any forks, and the commands would be more maintainable, and would gather the potential of so many Ruby developers, which if GitHub is any indication; a lot of them love Git. I think that's the only way forward, since the Git project doesn't wish to be improved. There's only one last patch series that I'll try to push to Git that I've been cooking for years. Sadly, I don't think it's going to land, despite it being clearly good, and a simple single patch doing the trick, and it would be immensely useful for our users, who have complained about that for years, and most Git developers have agreed it needs to be done. Yet, if I send it, it would not land. Perhaps it's time for me to create a pseudonym, and when you have to do that to land clearly good patches, you know something is *REALLY* wrong with the project. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] rm: introduce advice.rmHints to shorten messages
Le 2013-06-10 18:57, Junio C Hamano a écrit : Mathieu Liénard--Mayor mathieu.lienard--ma...@ensimag.fr writes: Please ignore this, manipulation error while in the git send-email command line. Here is what my mailbox looks like (the penultimate one with 252 lines is what I am responding to). R. [ 146: Mathieu Lienard--Mayor ] [PATCH 1/2] rm: better error messa R. [ 231: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce advice.r R. [ 157: Mathieu Lienard--Mayor ] [PATCH 2/2] rm: introduce advice.r R. [ 198: Mathieu Lienard--Mayor ] [PATCH v2 1/2] rm: better error me R. [ 157: Mathieu Lienard--Mayor ] [PATCH v2 2/2] rm: introduce advic . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce advic . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2] rm: better error m . [ 214: Mathieu Lienard--Mayor ] [PATCH v3 1/2] rm: better error m . [ 153: Mathieu Lienard--Mayor ] [PATCH v3 2/2] rm: introduce advic R [ 33: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error m O [ 38: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce advi R. [ 156: Mathieu Liénard--Mayor ] Re: [PATCH 1/2] rm: better error m R. [ 252: Mathieu Liénard--Mayor ] Re: [PATCH 2/2] rm: introduce advi . [ 84: Mathieu Liénard--Mayor ] Re: [PATCH v2 1/2] rm: better erro I am guessing that [v3 1/2] and [v3 2/2] are the final ones but it that is not the case please holler. Yes, [v3 1/2] and [v3 2/2] are the final ones. i'm sorry, i really don't know how i managed to create such a mess, i'm still not familiar with the send-email tool =/ -- Mathieu Liénard--Mayor, 2nd year at Grenoble INP - ENSIMAG (+33)6 80 56 30 02 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] rm: introduce advice.rmHints to shorten messages
Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
writes:
Introduce advice.rmHints to choose whether to display advice or not
when git rm fails. Defaults to true, in order to preserve current behavior.
As an example, the message:
error: 'foo.txt' has changes staged in the index
(use --cached to keep the file, or -f to force removal)
would look like, with advice.rmHints=false:
error: 'foo.txt' has changes staged in the index
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Documentation/config.txt |3 +++
advice.c |2 ++
advice.h |1 +
builtin/rm.c | 11 +++
t/t3600-rm.sh| 29 +
5 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..eb04479 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -199,6 +199,9 @@ advice.*::
amWorkDir::
Advice that shows the location of the patch file when
linkgit:git-am[1] fails to apply it.
+ rmHints::
+ In case of failure in the output of linkgit:git-rm[1],
+ show directions on how to proceed from the current state.
--
core.fileMode::
diff --git a/advice.c b/advice.c
index a8deee6..a4c169c 100644
--- a/advice.c
+++ b/advice.c
@@ -14,6 +14,7 @@ int advice_resolve_conflict = 1;
int advice_implicit_identity = 1;
int advice_detached_head = 1;
int advice_set_upstream_failure = 1;
+int advice_rm_hints = 1;
static struct {
const char *name;
@@ -33,6 +34,7 @@ static struct {
{ implicitidentity, advice_implicit_identity },
{ detachedhead, advice_detached_head },
{ setupstreamfailure, advice_set_upstream_failure },
+ { rmhints, advice_rm_hints },
/* make this an alias for backward compatibility */
{ pushnonfastforward, advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index 94caa32..36104c4 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ extern int advice_resolve_conflict;
extern int advice_implicit_identity;
extern int advice_detached_head;
extern int advice_set_upstream_failure;
+extern int advice_rm_hints;
The handling of a new advice variable (i.e. definition, declaration
and reading from configuration) looks correct in this patch. Good
job.
int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
diff --git a/builtin/rm.c b/builtin/rm.c
index 07306eb..c991fe6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -62,9 +62,11 @@ static int check_submodules_use_gitfiles(void)
if (!submodule_uses_gitfile(name))
errs = error(_(submodule '%s' (or one of its nested
+submodules) uses a .git directory%s),
name,
+advice_rm_hints
+? \n(use 'rm -rf' if you really want to
remove
+it including all of its history)
+: );
The advice part is not subject to i18n?
}
return errs;
Interesting.
Is there a reason why this kind of errors are not collected together
into one error message and then list of paths, like all the other
kinds of errors are done with print_eventual_error_files()?
@@ -83,7 +85,8 @@ static void print_eventual_error_files(struct string_list
*files_list,
strbuf_addf(err_msg,
\n%s,
files_list-items[i].string);
- strbuf_addstr(err_msg, hints_msg);
+ if (advice_rm_hints)
+ strbuf_addstr(err_msg, hints_msg);
*errs = error(%s, err_msg.buf);
}
}
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 10dd380..74f048c 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -707,6 +707,18 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rm files with different staged content without hints' '
+ cat expect EOF
+error: the following files have staged content different from both the
+file and the HEAD:
+bar.txt
+foo.txt
+EOF
+ echo content2 foo.txt
+ echo content2 bar.txt
+ test_must_fail git -c advice.rmhints=false rm foo.txt bar.txt 2actual
+ test_cmp expect actual
+'
Same comments as the ones for 1/2 applies to the tests in this patch.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] test: improve rebase -q test
On Mon, Jun 10, 2013 at 08:56:58AM -0700, Junio C Hamano wrote: SZEDER Gábor sze...@ira.uka.de writes: On Sun, Jun 09, 2013 at 03:41:54PM -0500, Felipe Contreras wrote: There will not be a need for test_string_must_be_empty() just like there's no need for test_string_cmp(). Actually, if there were a test_string_cmp(), that would be the test helper function I used most often. Hmm, there indeed are quite a many At this point, the variable's value must be this in the test scripts. With things like this: t/t0002-gitfile.sh: test $SHA = $(git rev-list HEAD) we can go to the trash directory upon seeing a failure to run the command used on the RHS, but the value in $SHA is cumbersome to find out (either running it under sh -x or insert an extra echo before it), so such a helper function may be useful. Do you really need a general comparison (does A sort before B) or just equality? If the latter, test_string_equal (or even string_equal) might be a better name for it. Yeah, I need only equality. Or at least it would be nice to have. My main motivation is that, like in your example, in the bash prompt tests I only have to check a single line of output, but because of debuggability I always did: echo (master) expected __git_ps1 actual test_cmp expected actual With such a helper function this could be reduced to a single line: test_string_equal (master) $(__git_ps1) without loss of functionality or debuggability, because in case of a failure it would output something like this (bikesheddable, of course): Error: expected: (master) got: ((deadbeef...)) And perhaps with a description as an optional third argument to help identify the failed check if multiple such checks are done in a single test, e.g. the test_rev_parse() helper in t/t1501-worktree.sh, 'setup: helper for testing rev-parse', which could be shortened as: test_string_equal $1 $(git rev-parse --is-bare-repository) bare test_string_equal $2 $(git rev-parse --is-inside-git-dir) gitdir test_string_equal $3 $(git rev-parse --is-inside-work-tree) worktree and if something goes wrong we'd get: Error: worktree expected: true got: false Perhaps I could find some time in the days ahead to give it a go. Gábor -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style
Please, don't top-post. Quote the part of the message you're replying to, and reply below. Benoît Person benoit.per...@ensimag.fr writes: Well, I think next step would be to replace all those calls with Git.pm `command`, `command_oneline` and `config``which take an array of arguments after the command. In the preview tool we use those but I don't know if we will find the time to clean that up too in git-remote-mediawiki :) . Agreed. run_git was written to avoid having to depend on Git.pm, but now that we do, we should do it the Git.pm way (although this is not a high priority for now). Don't know though if it's better to mix that with this serie which is mainly based on what perlcritic returns. If you go this way, I'd rather have it on top (i.e. a separate patch series). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Felipe Contreras felipe.contre...@gmail.com writes: It is not bad behavior. It is bad behavior *in your opinion*, And in essentially everyone else on this list, it seems. an opinion that wouldn't be shared by other projects, like the Linux kernel. Googling your name and LKML gives me this in the first page (addressed to you): https://lkml.org/lkml/2012/4/12/434 I'm stupider for just reading your email. Go away. https://lkml.org/lkml/2012/4/15/112 I'll make one more try at explaining to you, but then I'll just set my mail reader to ignore you, because judging by past performance (not just in this thread) you will just continue to argue. I don't follow the lkml so maybe I've just been unlucky and Google didn't show me an accurate sample, but arguing that your behavior is welcome on the LKML seems weird. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: v3 [PATCH 1/2] status: introduce status.short to enable --short by default
Matthieu Moy matthieu@grenoble-inp.fr writes: y...@ensimag.imag.fr writes: To: y...@ensimag.imag.fr Common mistake, but you're not supposed to answer y when you're prompted for an email ;-). Didn't we introduce safety against this in v1.7.12.1 and later? Is the new release taking more than 9 months to percolate, or are there still other codepaths that allow this to happen that we need to add further safety? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Move sequencer to builtin
Matthieu Moy wrote: https://lkml.org/lkml/2012/4/12/434 https://lkml.org/lkml/2012/4/15/112 We don't want things taken out of context now, do we? Follow up this thread [1], if you're interested in that discussion. I did clip out the quotes you chose on purpose, in the interest of presenting evidence in an unbiased manner. I don't follow the lkml so maybe I've just been unlucky and Google didn't show me an accurate sample, but arguing that your behavior is welcome on the LKML seems weird. Are people criticizing his discussion style, tone, and demeanour, instead of focusing on the argument? [1]: http://thread.gmane.org/gmane.linux.kernel/1280458/focus=8675 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bad attitudes and problems in the Git community (was: Re: [PATCH 2/2] Move sequencer to builtin)
On Mon, Jun 10, 2013 at 9:58 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Jun 10, 2013 at 4:05 AM, Stefano Lattarini stefano.lattar...@gmail.com wrote: You need two sides to have an argument. I disagree. Unless you mean than, whenever a part behaves in a hostile and aggressive way, the other part should just silently knuckle under. You are wrong. If a bum in the street starts talking about you about why you are going to hell, and you reply to him and argue. Who has the fault of starting an argument? I'm not sure I follow the analogy. Are you the bum or the passer-by? Sorry, couldn't help it. :-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] commit-queue: LIFO or priority queue of commits
On Mon, Jun 10, 2013 at 12:21:00AM -0700, Junio C Hamano wrote:
It may be worth looking again for other places to use this over
commit_list, but even the caller you are introducing here justifies its
presence.
The next candidate is paint-down-to-common, probably.
Yeah, I don't think I looked at that at all last time (mostly because it
only large as the graph gets wide, which is typically acceptable for
us). But it should be easy to do.
Also, I wrote some basic tests to cover the priority queue as a unit. I
can rebase them on your commit if you are interested.
It would be great.
Squashable patch is below.
Is it worth making this struct commit * a void pointer, and handling
arbitrary items in our priority queue? The compare function should be
the only thing that dereferences them.
I do not have any non-commit priority queue use in mind, but I do not
think it adds any complexity in this case.
I didn't either (and still I don't think of one), but I agree that
the implementation can be reused for pq of any type, as long as it
is a pointer to struct.
I converted this to a void pointer in my patch below, simply because it
makes it easier to write a test-queue that operates on ints. Due to
implicit casting, it should work for the most part without changing the
calling code unless you have a caller that does something like:
commit_queue_get(q)-date
or similar. I didn't change the name, either. It may be silly to call it
commit_queue still since it is now more general. I simply called mine
queue (I wanted pqueue, but that conflicted with globals defined by
OpenSSL; yours is a more general queue anyway, so maybe that is a good
name).
+ /* Bubble up the new one */
+ for (ix = queue-nr - 1; ix; ix = parent) {
+ parent = (ix - 1) / 2;
+ if (compare(queue-array[parent], queue-array[ix],
+ queue-cb_data) 0)
+ break;
In my implementation, I stopped on compare() = 0. It is late and my
mind is fuzzy, but I recall that heaps are never stable with respect to
insertion order, so I don't think it would matter.
It would matter in the sense that we cannot replace linked-list, if
the caller wants stability. It is more like we cannot do anything
about it than it would not matter.
Right. I meant I do not think it matters if you do = or here, as we
are not stable anyway. Doing = 0 stops the heapify operation sooner,
though I doubt it matters in practice (it is not an algorithmic
complexity change, but just that you can sometimes quit early).
I think it is the same situation in your push down, too, where you can
quit when the parent is equal to the largest child.
We can make each queue element a pair of pointer to payload,
insertion counter, and tiebreak using the insertion order, if the
callers want the same stability as linked-list implementation, but
I tend to think it really matters.
Yes, I think that is the usual solution.
Here's the patch with the tests, meant to be squashed into your 2/4. As
I mentioned above, you may want to further tweak the name, which would
require fixing up the rebase patches on top.
If you don't want to do the s/struct commit/void/ change now, we can
probably just have test-queue stuff the ints into commit pointers.
The tests themselves are not extremely extensive, but at least let you
check that you implemented the heap correctly. :)
---
.gitignore | 1 +
Makefile | 1 +
commit-queue.c | 6 ++---
commit-queue.h | 8 +++---
t/t0009-queue.sh | 50 +++
test-queue.c | 39 +++
6 files changed, 98 insertions(+), 7 deletions(-)
diff --git a/.gitignore b/.gitignore
index 6669bf0..8670e6d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -193,6 +193,7 @@
/test-regex
/test-revision-walking
/test-run-command
+/test-queue
/test-sha1
/test-sigchain
/test-string-list
diff --git a/Makefile b/Makefile
index 3cf55e9..c957637 100644
--- a/Makefile
+++ b/Makefile
@@ -552,6 +552,7 @@ TEST_PROGRAMS_NEED_X += test-path-utils
TEST_PROGRAMS_NEED_X += test-mktemp
TEST_PROGRAMS_NEED_X += test-parse-options
TEST_PROGRAMS_NEED_X += test-path-utils
+TEST_PROGRAMS_NEED_X += test-queue
TEST_PROGRAMS_NEED_X += test-regex
TEST_PROGRAMS_NEED_X += test-revision-walking
TEST_PROGRAMS_NEED_X += test-run-command
diff --git a/commit-queue.c b/commit-queue.c
index 77d4b02..04acf23 100644
--- a/commit-queue.c
+++ b/commit-queue.c
@@ -10,7 +10,7 @@ void clear_commit_queue(struct commit_queue *queue)
queue-array = NULL;
}
-void commit_queue_put(struct commit_queue *queue, struct commit *commit)
+void commit_queue_put(struct commit_queue *queue, void *commit)
{
commit_compare_fn compare = queue-compare;
int ix, parent;
@@ -34,9 +34,9 @@ struct commit *commit_queue_get(struct commit_queue *queue)
}
}
-struct commit *commit_queue_get(struct commit_queue *queue)
+void
Re: [PATCH v3 1/2] status: introduce status.short to enable --short by default
Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr
writes:
Some people always run 'git status -s'.
The configuration variable status.short allows to set it by default.
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v2:
-removal of double quotes in test
-use of git config --unset to clean up test environment
Documentation/config.txt |4
builtin/commit.c |5 +
t/t7508-status.sh| 35 +++
3 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53fc5..1983bf7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2066,6 +2066,10 @@ status.relativePaths::
relative to the repository root (this was the default for Git
prior to v1.5.4).
+status.short::
+ Set to true to enable --short by default in linkgit:git-status[1].
+ The option --no-short takes precedence over this variable.
+
status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 1621dfc..287f1cb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1112,6 +1112,11 @@ static int git_status_config(const char *k, const char
*v, void *cb)
s-submodule_summary = -1;
return 0;
}
+ if (!strcmp(k, status.short)) {
+ if (git_config_bool(k, v))
+ status_format = STATUS_FORMAT_SHORT;
And if the user has
[status]
short = no
in $GIT_DIR/config for this particular project, perhaps in order to
override a blanket setting
[status]
short
that is in $HOME/.gitconfig, what should happen?
Perhaps you need
if (git_config_bool(...))
status_format = STATUS_FORMAT_SHORT;
else
status_format = STATUS_FORMAT_NONE; /* default */
or something here?
+ return 0;
+ }
if (!strcmp(k, status.color) || !strcmp(k, color.status)) {
s-use_color = git_config_colorbool(k, v);
return 0;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index e2ffdac..d99ca9f 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1335,4 +1335,39 @@ test_expect_failure '.git/config ignore=all suppresses
submodule summary' '
git config -f .gitmodules --remove-section submodule.subname
'
+test_expect_success 'Setup of test environment' '
+ git config status.showUntrackedFiles no
+'
+
+test_expect_success 'status.short=true same as -s' '
+ git -c status.short=true status actual
+ git status -s expected_short
+ test_cmp actual expected_short
+'
+
+test_expect_success 'status.short=true different from --no-short' '
+ git status --no-short expected_noshort
+ test_must_fail test_cmp actual expected_noshort
+'
+
+test_expect_success 'status.short=true weaker than --no-short' '
+ git -c status.short=true status --no-short actual
+ test_cmp actual expected_noshort
+'
+
+test_expect_success 'status.short=false same as --no-short' '
+ git -c status.short=false status actual
+ git status -s expected_short
+ test_cmp actual expected_noshort
+'
+
+test_expect_success 'status.short=false weaker than -s' '
+ git -c status.short=false status -s actual
+ test_cmp actual expected_short
+'
+
+test_expect_success 'Restore default test environment' '
+ git config --unset status.showUntrackedFiles
+'
A few observations.
* It is very good that you check not just positive cases that show
off how well this new feature works (i.e. status.short set
without command line override gives a short output) but also
negative cases that make sure the new feature does not kick in
when it should not. You test all four combinations, which is
good.
* If any of the first three fails, you may not have the correct
string in expected_short or expected_noshort when running later
tests that depend on them.
* Similarly, if the first one to set showUntrackedFiles fails, the
last one to --unset would also fail.
Perhaps limiting the number of tests that must pass (otherwise the
remainder becomes useless) by doing something like this is a better
alternative:
test_expect_success 'setup for status.short' '
git status --short expected_short
git status --no-short expected_noshort
'
test_expect_success '-c status.short=true == status -s' '
test_config status.showUntrackedFile no
test_config
Re: [PATCH v3 2/2] status:introduce status.branch to enable --branch by default
Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr
writes:
Some people often run 'git status -b'.
The config variable status.branch allows to set it by default.
Signed-off-by: Jorge Juan Garcia Garcia
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
Changes since v2:
-removal of double quotes in test
Documentation/config.txt |4
builtin/commit.c |4
t/t7508-status.sh| 27 +++
3 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1983bf7..ecdcd6d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2070,6 +2070,10 @@ status.short::
Set to true to enable --short by default in linkgit:git-status[1].
The option --no-short takes precedence over this variable.
+status.branch::
+ Set to true to enable --branch by default in linkgit:git-status[1].
+ The option --no-branch takes precedence over this variable.
+
status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 287f1cb..f2b5d44 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1117,6 +1117,10 @@ static int git_status_config(const char *k, const char
*v, void *cb)
status_format = STATUS_FORMAT_SHORT;
return 0;
}
+ if (!strcmp(k, status.branch)) {
+ s-show_branch = git_config_bool(k, v);
This one, unlike 1/2, acts correctly when status.branch is set to
no. Good.
The same comments as 1/2 apply to the test script additions in this
patch.
+ return 0;
+ }
if (!strcmp(k, status.color) || !strcmp(k, color.status)) {
s-use_color = git_config_colorbool(k, v);
return 0;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index d99ca9f..5e6df95 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1366,6 +1366,33 @@ test_expect_success 'status.short=false weaker than
-s' '
test_cmp actual expected_short
'
+test_expect_success 'status.branch=true same as -b' '
+ git -c status.branch=true status -s actual
+ git status -sb expected_branch
+ test_cmp actual expected_branch
+'
+
+test_expect_success 'status.branch=true different from --no-branch' '
+ git -c status.branch=true status -s actual
+ git status -s --no-branch expected_nobranch
+ test_must_fail test_cmp actual expected_nobranch
+'
+
+test_expect_success 'status.branch=true weaker than --no-branch' '
+ git -c status.branch=true status -s --no-branch actual
+ test_cmp actual expected_nobranch
+'
+
+test_expect_success 'status.branch=false same as --no-branch' '
+ git -c status.branch=false status -s actual
+ test_cmp actual expected_nobranch
+'
+
+test_expect_success 'status.branch=false weaker than -b' '
+ git -c status.branch=false status -sb actual
+ test_cmp actual expected_branch
+'
+
test_expect_success 'Restore default test environment' '
git config --unset status.showUntrackedFiles
'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] sort-in-topological-order: use commit-queue
On Mon, Jun 10, 2013 at 12:27:31AM -0700, Junio C Hamano wrote: Around the same time, though, René wrote the linked-list merge sort that powers commit_list_sort_by_date. And topo-sort learned to do O(1) insertions into the unsorted list, and then one O(n log n) sort. Yes, but that only affects the sort the work queue in date order before entering the main loop, and maintenance of work queue as we dig along still is find the place to put this in the date-order sorted linked list, no? Ah, you're right. I was thinking that we saw all of the commits up front and then sorted. And we do, but we still keep a separate list in the work queue. So I think it may just be the case that N does not get very big here (the width of the graph), so log(N) versus (N) does not make a big difference. I've been disturbed every time I saw the commit_list insertion function that does a small allocation which will be freed fairly often and have been wondering if we can rewrite it with custom slab allocator, but not using linked list where we do not have to feels like a better solution to that issue, and use of pqueue may be a right direction to go in. Agreed. The only thing I'd worry about is that somebody cares about the order stability of same-time commits in the output. But I cannot think of a case where it is important (especially because the timestamps are subject to minor skew anyway, so it is not like you could even count on particular commits having equivalent timestamps). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] status: introduce status.short to enable --short by default
Junio C Hamano gits...@pobox.com writes: test_expect_success '-c status.short=true == status -s' ' test_config status.showUntrackedFile no That's an option, but having status.showUntrackedFile set in a separate setup test makes the actual tests shorter. The setup test has no reason to fail, so I find it OK. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t/checkout-last: checkout - doesn't work after rebase -i
Ramkumar Ramachandra artag...@gmail.com writes: The following command $ git checkout - does not work as expected after a 'git rebase -i'. Add a failing test documenting this bug. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t2012-checkout-last.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh index b44de9d..5729487 100755 --- a/t/t2012-checkout-last.sh +++ b/t/t2012-checkout-last.sh @@ -116,4 +116,12 @@ test_expect_success 'master...' ' test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify master^) ' +test_expect_failure 'checkout - works after an interactive rebase' ' + git checkout master + git checkout other + git rebase -i master + git checkout - + test z$(git symbolic-ref HEAD) = zrefs/heads/master +' Hmph, you were on other and then ran rebase -i to rebase it. When you are done, you are on other. You want to go back to the one before you checked out the branch you started your rebase -i on, which is master. OK, the expectation makes sense to me. These four are all valid ways to spell the rebase -i master step. git rebase -i master git rebase -i --onto master master git rebase -i master other git rebase -i --onto master master other and I think it is sensible to expect (1) they all behave the same way; or (2) the first two behave the same, but the latter two explicitly checks out 'other' by giving the last argument. If you are not on 'other' when you run the rebase -i, checkout - should come back to the branch before 'other', i.e. the branch you started your rebase -i on. In other words, (2) would mean: git checkout master git checkout -b naster git rebase -i master other # ends up on other test_string_equal $(git symbolic-ref HEAD) refs/heads/other git checkout - # we were on naster before we rebased 'other' test_string_equal $(git symbolic-ref HEAD) refs/heads/naster It is a bit unclear what the following should expect. git checkout master git checkout other git rebase -i master other # ends up on other test_string_equal $(git symbolic-ref HEAD) refs/heads/other git checkout - # we are on ??? before we rebased other test_string_equal $(git symbolic-ref HEAD) refs/heads/??? One could argue that the first checkout other and then rebase done by rebase becomes a no-op and the user should be aware of that because rebase is started while on that other branch, in which case we should come back to 'master'. From consistency point of view, one could instead argue that we were on 'other' before we rebased it, in which case it may not be unreasonable for checkout - to come back to 'other'. I tend to prefer the former (i.e. go back to 'master') over the latter but not by a large margin. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] checkout: respect GIT_REFLOG_ACTION
Ramkumar Ramachandra artag...@gmail.com writes:
GIT_REFLOG_ACTION is an environment variable specifying the reflog
message to write after an action is completed. Other commands including
merge, reset, and commit respect it.
This incidentally fixes a bug in t/checkout-last. You can now expect
$ git checkout -
to work fine after an interactive rebase.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
builtin/checkout.c | 11 ---
t/t2012-checkout-last.sh | 2 +-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f5b50e5..1e2af85 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -587,7 +587,7 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
struct branch_info *new)
{
struct strbuf msg = STRBUF_INIT;
- const char *old_desc;
+ const char *old_desc, *reflog_msg;
if (opts-new_branch) {
if (opts-new_orphan_branch) {
if (opts-new_branch_log !log_all_ref_updates) {
@@ -620,8 +620,13 @@ static void update_refs_for_switch(const struct
checkout_opts *opts,
old_desc = old-name;
if (!old_desc old-commit)
old_desc = sha1_to_hex(old-commit-object.sha1);
- strbuf_addf(msg, checkout: moving from %s to %s,
- old_desc ? old_desc : (invalid), new-name);
+
+ reflog_msg = getenv(GIT_REFLOG_ACTION);
+ if (!reflog_msg)
+ strbuf_addf(msg, checkout: moving from %s to %s,
+ old_desc ? old_desc : (invalid), new-name);
+ else
+ strbuf_insert(msg, 0, reflog_msg, strlen(reflog_msg));
Looks very sensible; we may need to audit programs that set and
export REFLOG_ACTION to make sure they do not do so incorrectly,
which wouldn't have mattered if they called checkout but now it
would with this fix, though.
if (!strcmp(new-name, HEAD) !new-path !opts-force_detach) {
/* Nothing to do. */
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index 5729487..ab80da7 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,7 +116,7 @@ test_expect_success 'master...' '
test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify
master^)
'
-test_expect_failure 'checkout - works after an interactive rebase' '
+test_expect_success 'checkout - works after an interactive rebase' '
git checkout master
git checkout other
git rebase -i master
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] rebase -i: write better reflog messages for start
Ramkumar Ramachandra artag...@gmail.com writes: Invoking 'git rebase -i' writes the following line to the reflog at the start of the operation: rebase -i (start) This is not very useful. Make it more informative like: rebase -i (start): checkout master Makes sense to me, at least within the scope of the patch context. I am curious what breaks, though. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- git-rebase--interactive.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 5822b2c..a05a6e4 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -837,6 +837,7 @@ comment_for_reflog start if test ! -z $switch_to then + GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $switch_to output git checkout $switch_to -- || die Could not checkout $switch_to fi @@ -980,6 +981,7 @@ has_action $todo || test -d $rewritten || test -n $force_rebase || skip_unnecessary_picks +GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $onto_name output git checkout $onto || die_abort could not detach HEAD git update-ref ORIG_HEAD $orig_head do_rest -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] Documentation: Update manpage for pre-commit hook
Signed-off-by: Richard Hartmann richih.mailingl...@gmail.com --- Documentation/githooks.txt |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index b9003fe..1276730 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -80,7 +80,8 @@ causes the 'git commit' to abort. The default 'pre-commit' hook, when enabled, catches introduction of lines with trailing whitespaces and aborts the commit when -such a line is found. +such a line is found. It will also prevent addition of non-ASCII +file names. All the 'git commit' hooks are invoked with the environment variable `GIT_EDITOR=:` if the command will not bring up an editor -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] template: Fix comment indentation in pre-rebase hook
The other hooks use two whitespace for indentation instead of tabs to signify code in the example/echo output. Follow the same layout in templates/hooks--pre-rebase.sample Signed-off-by: Richard Hartmann richih.mailingl...@gmail.com --- templates/hooks--pre-rebase.sample | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/hooks--pre-rebase.sample b/templates/hooks--pre-rebase.sample index b74cd1d..43426e0 100755 --- a/templates/hooks--pre-rebase.sample +++ b/templates/hooks--pre-rebase.sample @@ -157,13 +157,13 @@ B to be deleted. To compute (1): - git rev-list ^master ^topic next - git rev-list ^masternext + git rev-list ^master ^topic next + git rev-list ^masternext - if these match, topic has not merged in next at all. + if these match, topic has not merged in next at all. To compute (2): - git rev-list master..topic + git rev-list master..topic - if this is empty, it is fully merged to master. + if this is empty, it is fully merged to master. -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] templates: Reformat pre-commit hook's message
Now that the there's only one echo being spawned, the message can span the full 80 chars. Signed-off-by: Richard Hartmann richih.mailingl...@gmail.com --- templates/hooks--pre-commit.sample |6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample index 126ae13..7676c6e 100755 --- a/templates/hooks--pre-commit.sample +++ b/templates/hooks--pre-commit.sample @@ -33,13 +33,11 @@ if [ $allownonascii != true ] then echo 'Error: Attempt to add a non-ascii file name. -This can cause problems if you want to work -with people on other platforms. +This can cause problems if you want to work with people on other platforms. To be portable it is advisable to rename the file. -If you know what you are doing you can disable this -check using: +If you know what you are doing you can disable this check using: git config hooks.allownonascii true ' -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] templates: Fix ASCII art in pre-rebase hook
The example assumes 8-char wide tabs and breaks for people with 4-char wide tabs. Signed-off-by: Richard Hartmann richih.mailingl...@gmail.com --- templates/hooks--pre-rebase.sample | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/templates/hooks--pre-rebase.sample b/templates/hooks--pre-rebase.sample index 053f111..b74cd1d 100755 --- a/templates/hooks--pre-rebase.sample +++ b/templates/hooks--pre-rebase.sample @@ -132,14 +132,14 @@ With this workflow, you would want to know: Let's look at this example: - o---o---o---o---o---o---o---o---o---o next - / / / / -/ a---a---b A / / - / / / / - / / c---c---c---c B / - / / / \ / -/ / / b---b C \ / - / / / / \ / + o---o---o---o---o---o---o---o---o---o next + / / / / + / a---a---b A / / +/ / / / + / / c---c---c---c B / + / / / \ / + / / / b---b C \ / +/ / / / \ / ---o---o---o---o---o---o---o---o---o---o---o master -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] templates: Fix spelling in pre-commit hook
Signed-off-by: Richard Hartmann richih.mailingl...@gmail.com --- templates/hooks--pre-commit.sample |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample index 7676c6e..a982d99 100755 --- a/templates/hooks--pre-commit.sample +++ b/templates/hooks--pre-commit.sample @@ -15,13 +15,13 @@ else against=4b825dc642cb6eb9a060e54bf8d69288fbee4904 fi -# If you want to allow non-ascii filenames set this variable to true. +# If you want to allow non-ASCII filenames set this variable to true. allownonascii=$(git config hooks.allownonascii) # Redirect output to stderr. exec 12 -# Cross platform projects tend to avoid non-ascii filenames; prevent +# Cross platform projects tend to avoid non-ASCII filenames; prevent # them from being added to the repository. We exploit the fact that the # printable range starts at the space character and ends with tilde. if [ $allownonascii != true ] @@ -31,7 +31,7 @@ if [ $allownonascii != true ] test $(git diff --cached --name-only --diff-filter=A -z $against | LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 then - echo 'Error: Attempt to add a non-ascii file name. + echo 'Error: Attempt to add a non-ASCII file name. This can cause problems if you want to work with people on other platforms. -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] templates: Fewer subprocesses in pre-commit hook
Spawning a new subprocess for every line printed is inefficient. Thus spawn only one instance of `echo`. Signed-off-by: Richard Hartmann richih.mailingl...@gmail.com --- templates/hooks--pre-commit.sample | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample index 18c4829..126ae13 100755 --- a/templates/hooks--pre-commit.sample +++ b/templates/hooks--pre-commit.sample @@ -31,18 +31,18 @@ if [ $allownonascii != true ] test $(git diff --cached --name-only --diff-filter=A -z $against | LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 then - echo Error: Attempt to add a non-ascii file name. - echo - echo This can cause problems if you want to work - echo with people on other platforms. - echo - echo To be portable it is advisable to rename the file ... - echo - echo If you know what you are doing you can disable this - echo check using: - echo - echo git config hooks.allownonascii true - echo + echo 'Error: Attempt to add a non-ascii file name. + +This can cause problems if you want to work +with people on other platforms. + +To be portable it is advisable to rename the file. + +If you know what you are doing you can disable this +check using: + + git config hooks.allownonascii true +' exit 1 fi -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Janitorial work on hook templates
Dear all, attached, you will find a series of small, obvious, and hopefully uncontroversial patches for the hook templates and the manpage. They are all tiny, but I still decided to submit distinct patches to make modification/discussion easier. Richard Hartmann (6): templates: Fewer subprocesses in pre-commit hook templates: Reformat pre-commit hook's message templates: Fix spelling in pre-commit hook Documentation: Update manpage for pre-commit hook templates: Fix ASCII art in pre-rebase hook template: Fix comment indentation in pre-rebase hook Documentation/githooks.txt |3 ++- templates/hooks--pre-commit.sample | 26 -- templates/hooks--pre-rebase.sample | 26 +- 3 files changed, 27 insertions(+), 28 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] rebase -i: write better reflog messages for start
Junio C Hamano wrote: I am curious what breaks, though. t/status-help. Looks seriously unrelated, and I'm breaking my head over it. Any clues? --- expected2013-06-10 17:16:42.276356867 + +++ actual 2013-06-10 17:16:42.279690201 + @@ -1,4 +1,4 @@ -# HEAD detached at 000106f +# HEAD detached from 88a81b6 # You are currently rebasing branch 'rebase_conflicts' on '000106f'. # (fix conflicts and then run git rebase --continue) # (use git rebase --skip to skip this patch) not ok 5 - status when rebase in progress before resolving conflicts -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bad attitudes and problems in the Git community (was: Re: [PATCH 2/2] Move sequencer to builtin)
Yes, sorry. I find this whole story quite amusing (albeit distracting and unnecessary), but sorry for adding to the spam. I'll be quiet now. On Mon, Jun 10, 2013 at 11:33 AM, Martin Langhoff martin.langh...@gmail.com wrote: On Mon, Jun 10, 2013 at 2:11 PM, Martin von Zweigbergk martinv...@gmail.com wrote: On Mon, Jun 10, 2013 at 9:58 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Mon, Jun 10, 2013 at 4:05 AM, Stefano Lattarini stefano.lattar...@gmail.com wrote: You need two sides to have an argument. I disagree. Unless you mean than, whenever a part behaves in a hostile and aggressive way, the other part should just silently knuckle under. You are wrong. If a bum in the street starts talking about you about why you are going to hell, and you reply to him and argue. Who has the fault of starting an argument? I'm not sure I follow the analogy. Are you the bum or the passer-by? http://xkcd.com/386/ Someone is wrong on the Internet! Let it be. m -- martin.langh...@gmail.com - ask interesting questions - don't get distracted with shiny stuff - working code first ~ http://docs.moodle.org/en/User:Martin_Langhoff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] log: --author-date-order
On Mon, Jun 10, 2013 at 12:39:24AM -0700, Junio C Hamano wrote:
I'm not excited about introducing yet another place that parses commit
objects (mostly not for correctness, but because we have had
inconsistency in how malformed objects are treated). It is at least
using split_ident_line which covers the hard bits. I wonder how much
slower it would be to simply call format_commit_message to do the
parsing.
The thought certainly crossed my mind, not exactly in that form but
more about splitting the machinery used in pretty.c into a more
reusable form.
The result of my attempt however did not become all that reusable
(admittedly I didn't spend too much brain cycles on it), so I punted
;-).
Yes, I feel like it has been tried before. The problem is that a clean
interface would let you get individual pieces of information with a
single call. But an efficient interface will utilize the same parsing
pass to get multiple items out, and stop parsing when we have gotten all
required items (but leave the parser in a consistent state so that we
can pick it up later).
The format_commit_one parser does that, but the format_commit_context
it holds is a bit bulky. I think it might be possible to pull out the
parsing bits into a separate struct, and you could call it something
like:
struct commit_parser parser;
unsigned long authordate;
const char *authorname;
int authorlen;
commit_parser_init(parser, commit);
authordate = commit_parse_authordate(parser);
authorname = commit_parse_authorname(parser, authorlen);
where the second parse call is basically free, because we've already
done (and cached) the hard work in the first call.
So they might look like:
static void parse_author_ident(struct commit_parser *parser)
{
if (!parser-author.name_begin) {
if (!parser-authorline.start)
parse_commit_header(parser);
split_ident_line(parser-author,
parser-authorline.start,
parser-authorline.len);
}
}
unsigned long commit_parse_authordate(struct commit_parser *parser)
{
parse_author_ident(parser);
/* XXX should check for malformedness here */
return strtoul(ident.date_begin, NULL, 10);
}
const char *commit_parse_authorname(struct commit_parser *parser,
unsigned long *len)
{
parse_author_ident(parser);
*len = parser.author.name_end - parser.author.name_begin;
return parser.author.name_begin;
}
and so forth. It would be easy (and have the same efficiency) for
format_commit_message to build on that, and it calling it from regular
code is not too bad.
But you are right. The commit-buffer may no longer be there, and
the --author-date-order option needs to read the object again
in this codepath. That would be in line with what --pretty/format
would do, I guess.
Or we could extend parse_commit() API to take an optional commit
info slab to store not just author date but other non-essential
stuff like people's names, and we arrange that extended API to be
triggered when we know --author-date-order is in effect?
I like the latter option. It takes a non-trivial amount of time to load
the commits from disk, and now we are potentially doing it 2 or 3 times
for a run (once to parse, once to get the author info for topo-sort, and
possibly later to show it if --pretty is given; though I did not check
and maybe we turn off save_commit_buffer with --pretty). It would be
nice to have an extended parse_object that handled that. I'm not sure of
the interface. Maybe variadic with pairs of type/slab, like:
parse_commit_extended(commit,
PARSE_COMMIT_AUTHORDATE, authordate_slab,
PARSE_COMMIT_DONE);
?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] commit-queue: LIFO or priority queue of commits
Jeff King p...@peff.net writes: On Mon, Jun 10, 2013 at 12:21:00AM -0700, Junio C Hamano wrote: It may be worth looking again for other places to use this over commit_list, but even the caller you are introducing here justifies its presence. The next candidate is paint-down-to-common, probably. Yeah, I don't think I looked at that at all last time (mostly because it only large as the graph gets wide, which is typically acceptable for us). But it should be easy to do. Also, I wrote some basic tests to cover the priority queue as a unit. I can rebase them on your commit if you are interested. It would be great. Squashable patch is below. Is it worth making this struct commit * a void pointer, and handling arbitrary items in our priority queue? The compare function should be the only thing that dereferences them. I do not have any non-commit priority queue use in mind, but I do not think it adds any complexity in this case. I didn't either (and still I don't think of one), but I agree that the implementation can be reused for pq of any type, as long as it is a pointer to struct. I converted this to a void pointer in my patch below, simply because it makes it easier to write a test-queue that operates on ints. Due to implicit casting, it should work for the most part without changing the calling code unless you have a caller that does something like: commit_queue_get(q)-date or similar. I didn't change the name, either. It may be silly to call it commit_queue still since it is now more general. I simply called mine queue (I wanted pqueue, but that conflicted with globals defined by OpenSSL; yours is a more general queue anyway, so maybe that is a good name). I agree that it makes sense not to call it either commit-queue or pqueue. While at it, the filenames should probably be moved as well, no? Here's the patch with the tests, meant to be squashed into your 2/4. As I mentioned above, you may want to further tweak the name, which would require fixing up the rebase patches on top. If you don't want to do the s/struct commit/void/ change now, we can probably just have test-queue stuff the ints into commit pointers. The tests themselves are not extremely extensive, but at least let you check that you implemented the heap correctly. :) Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/4] commit-queue: LIFO or priority queue of commits
On Mon, Jun 10, 2013 at 11:56:33AM -0700, Junio C Hamano wrote: or similar. I didn't change the name, either. It may be silly to call it commit_queue still since it is now more general. I simply called mine queue (I wanted pqueue, but that conflicted with globals defined by OpenSSL; yours is a more general queue anyway, so maybe that is a good name). I agree that it makes sense not to call it either commit-queue or pqueue. While at it, the filenames should probably be moved as well, no? Yeah, definitely. I left all of that as an exercise for you, since the name change will involve a lot of fallout in the other patches. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
