Re: [PATCH v2 0/3] Towards a useable git-branch
Duy Nguyen wrote: Nobody should ever parse these output with scripts. The color can be generated from color.branch.*. How do we implement color.branch.(current|local|remote|plain)? In the current code, we take a crude approach by hand-constructing argc, argv strings and passing it to cmd_for_each_ref(). There are no conditionals in the format syntax (and introducing one is probably not a good idea either): so, I'm guessing these configuration variables have to be read by for-each-ref? A bigger problem is show_detached(), --[no-]merged, --with and --contains. We need to move some of those code over to for-each-ref. I saw that you fixed this. Another problem is the new branch -v seems to less responsive than old branch -v because (I think) of sorting, even if we don't need it. Does your track-responsiveness patch fix this? I checked out your branch, made some more updates and pushed out to my for-each-ref-pretty again. Changes are: *pants* Sorry, I'm finding it hard to keep up. -- 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 0/3] Towards a useable git-branch
On Tue, Jun 4, 2013 at 7:52 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Duy Nguyen wrote: Nobody should ever parse these output with scripts. The color can be generated from color.branch.*. How do we implement color.branch.(current|local|remote|plain)? In the current code, we take a crude approach by hand-constructing argc, argv strings and passing it to cmd_for_each_ref(). There are no conditionals in the format syntax (and introducing one is probably not a good idea either): so, I'm guessing these configuration variables have to be read by for-each-ref? Maybe. I don't really like the idea that for-each-ref reads _branch_ config. We could introduce the same set of keys but in color.for-each-ref namespace. %C(auto) will take care of the logic and choose the correct color key. When we replace branch's listing code with for-each-ref, I think we could somehow override for-each-ref keys with branch ones in-core. Too hacky? A bigger problem is show_detached(), --[no-]merged, --with and --contains. We need to move some of those code over to for-each-ref. I saw that you fixed this. Nope. --[no-]merged and --contains seem easy. show_detached is still WIP, mostly because detached HEAD may or may not show when you provide a pattern to git-branch (e.g. git branch --list 'foo*') and because HEAD is always the first item, regardless of sorting order. get_head_description also seems more porcelain-ish than a plumbing feature.. Another problem is the new branch -v seems to less responsive than old branch -v because (I think) of sorting, even if we don't need it. Does your track-responsiveness patch fix this? Yes. I checked out your branch, made some more updates and pushed out to my for-each-ref-pretty again. Changes are: *pants* Sorry, I'm finding it hard to keep up. Sorry that branch was in a better shape the day I sent my previous email. Then I kind of used it as a playground with --[no-]merged, --contains and stuff :-P -- Duy -- 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 0/3] Towards a useable git-branch
Duy Nguyen wrote: It's because you don't pad enough spaces after %(refname:short) so the starting point of %(upstream:short) on each line is already unaligned, I think. Yeah, my stupidity. I really meant %|(30), and that works fine. -- 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 0/3] Towards a useable git-branch
On Tue, May 28, 2013 at 9:24 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Oh, and by the way: We're pretty close we are to replacing branch -v and branch -vv. brv = for-each-ref --format='%(HEAD) %C(green)%(*)%(refname:short)%C(reset) %(*)%(objectname:short) %(subject)' refs/heads brvv = for-each-ref --format='%(HEAD) %C(green)%(*)%(refname:short)%C(reset) %(*)%(objectname:short) %C(blue)%(upstream:short)%C(reset) %(subject)' refs/heads There are small differences: 1. In branch -v, the green-color of the branch name is dependent on %(HEAD). Not worth ironing out, in my opinion. 2. In branch -vv, there are dependent square braces that come on when %(refname:short) is set. We might want to introduce an undocumented %(refname:branchvv) for internal use by branch -vv, for backward compatibility. What do you think? I think we can change -v and -vv format slightly as long as the information remains the same. Nobody should ever parse these output with scripts. The color can be generated from color.branch.*. A bigger problem is show_detached(), --[no-]merged, --with and --contains. We need to move some of those code over to for-each-ref. Another problem is the new branch -v seems to less responsive than old branch -v because (I think) of sorting, even if we don't need it. I checked out your branch, made some more updates and pushed out to my for-each-ref-pretty again. Changes are: - fix segfault with for-each-ref --format=%something refs/tags/' - add --pretty for new format syntax and keep --format unchanged - add --[no-]column to for-each-ref (for git branch with no arguments) - remove branch listing code and use for-each-ref instead (41 inserts, 378 deletes, beautiful). - add --sort and --count to git-branch -- Duy -- 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 0/3] Towards a useable git-branch
Hi Duy, I just woke up and started looking at the series: it's rather well done, and I'm confident that this is the way forward. To reciprocate, I've done some work at gh:artagnon/git for-each-ref-pretty. See: https://github.com/artagnon/git/commits/for-each-ref-pretty There is one major problem though: %(N) doesn't work properly with f-e-r, and I'm not sure why. I'm not talking about your last patch where you compute * -- that works fine; it's just that %(N) doesn't when N is a concrete number. Also, a couple of minor annoyances: 1. When f-e-r is invoked with refs/tags, we get stray output. Atleast it doesn't segfault, thanks to your ignore-commit patch. Maybe printing stray output is the right thing to do, as opposed to erroring out. 2. %(*) only works with f-e-r atoms, not with pretty-format atoms. This is ofcourse obvious from the implementation, but isn't it a little consistent? Should we start off a new pretty-ref-formats document, where we explicitly exclude things like %ae (because of the hex overriding thing)? I don't think it's a problem if documented properly. -- 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 0/3] Towards a useable git-branch
Oh, and by the way: We're pretty close we are to replacing branch -v and branch -vv. brv = for-each-ref --format='%(HEAD) %C(green)%(*)%(refname:short)%C(reset) %(*)%(objectname:short) %(subject)' refs/heads brvv = for-each-ref --format='%(HEAD) %C(green)%(*)%(refname:short)%C(reset) %(*)%(objectname:short) %C(blue)%(upstream:short)%C(reset) %(subject)' refs/heads There are small differences: 1. In branch -v, the green-color of the branch name is dependent on %(HEAD). Not worth ironing out, in my opinion. 2. In branch -vv, there are dependent square braces that come on when %(refname:short) is set. We might want to introduce an undocumented %(refname:branchvv) for internal use by branch -vv, for backward compatibility. What do you think? -- 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 0/3] Towards a useable git-branch
Ramkumar Ramachandra wrote: %(N) doesn't work properly with f-e-r, and I'm not sure why. I'm not talking about your last patch where you compute * -- that works fine; it's just that %(N) doesn't when N is a concrete number. Try this: %(refname:short)%(30)%(upstream:short) (assuming that you have lots of branches). I'm noticing random alignment problems. However, %(N) doesn't seem to have this problem: %(30)%(refname:short)%(upstream:short) I'm not able to figure this 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 v2 0/3] Towards a useable git-branch
On Tue, May 28, 2013 at 9:28 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Ramkumar Ramachandra wrote: %(N) doesn't work properly with f-e-r, and I'm not sure why. I'm not talking about your last patch where you compute * -- that works fine; it's just that %(N) doesn't when N is a concrete number. Try this: %(refname:short)%(30)%(upstream:short) (assuming that you have lots of branches). I'm noticing random alignment problems. It's because you don't pad enough spaces after %(refname:short) so the starting point of %(upstream:short) on each line is already unaligned, I think. Try this: %(*)%(refname:short)%(30)%(upstream:short) or if you prefer at specific column (e.g. align upstream close to the 60th column, regardless of refname's length): %(refname:short)%|(60)%(upstream:short) -- Duy -- 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 0/3] Towards a useable git-branch
On Tue, May 28, 2013 at 9:01 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Also, a couple of minor annoyances: 1. When f-e-r is invoked with refs/tags, we get stray output. Atleast it doesn't segfault, thanks to your ignore-commit patch. Maybe printing stray output is the right thing to do, as opposed to erroring out. What format did you use? 2. %(*) only works with f-e-r atoms, not with pretty-format atoms. This is ofcourse obvious from the implementation, but isn't it a little consistent? It is not. I think it's documented as well as a known implementation limitation. It's not hard to be fixed (we could call format_commit_message again for all entries, this time with a single placeholder, to collect the best width). If anybody does need %(*)%H to work, we could do it. BTW, the way I implement get_atom_width() is not really optimal. For n lines, we call print_value() in get_atom_width n^2 times. We could cache the calculated width instead and reduce that to n times. Should we start off a new pretty-ref-formats document, where we explicitly exclude things like %ae (because of the hex overriding thing)? I don't think it's a problem if documented properly. And this one is doucmented as well, I think. I'd really like to introduce a new --pretty option (or something) that does not accept %xx as a hex notion, so %ae and friends won't be hidden. It's also a good thing for compatibility because currently %H in --format produces %H. After this, %H produces something else. It's unlikely that anybody has done that. But it's even better if we avoid that possibility entirely with a new option. -- Duy -- 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 0/3] Towards a useable git-branch
On Sat, May 25, 2013 at 5:51 AM, Duy Nguyen pclo...@gmail.com wrote: I just had an idea that might bring pretty stuff to for-each-ref with probably reasonable effort, making for-each-ref format a superset of pretty. But I need to clean up my backlog first. Give me a few days, I will show you something (or give up ;) And I can't get it out of my head. Might as well write it down. Check out https://github.com/pclouds/git.git for-each-ref-pretty It opens a hook in format_commit_message() to plug f-e-r's atom syntax in. I didn't do extensive test or anything, just t6300. The %xx syntax in for-each-ref might override some placeholders in pretty, I did not check. You can add extra %(xx) on top as you have done. I still need one more hook to support %(*) with automatic width detection. After that I'm quite confident we can kill -v/-vv code. -- Duy -- 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 0/3] Towards a useable git-branch
On Sat, May 25, 2013 at 1:26 PM, Duy Nguyen pclo...@gmail.com wrote: On Sat, May 25, 2013 at 5:51 AM, Duy Nguyen pclo...@gmail.com wrote: I just had an idea that might bring pretty stuff to for-each-ref with probably reasonable effort, making for-each-ref format a superset of pretty. But I need to clean up my backlog first. Give me a few days, I will show you something (or give up ;) And I can't get it out of my head. Might as well write it down. Check out https://github.com/pclouds/git.git for-each-ref-pretty Ram, fetch the url above again. Its tip now is 5b4aa27 (for-each-ref: introduce format specifier %(*) and %(*) - 2013-05-25). Those changes make for-each-ref --format a superset of pretty. You can add new %(xxx) on top and resend the whole thing to the list for review. You can remove branch -v/-vv code as well or I'll add some patches on top to do that later. I have some compatibility concerns about the superset thing. But let's wait until the series hits git@vger. -- Duy -- 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 0/3] Towards a useable git-branch
Duy Nguyen wrote: Ram, fetch the url above again. Its tip now is 5b4aa27 (for-each-ref: introduce format specifier %(*) and %(*) - 2013-05-25). Those changes make for-each-ref --format a superset of pretty. You can add new %(xxx) on top and resend the whole thing to the list for review. You can remove branch -v/-vv code as well or I'll add some patches on top to do that later. I have some compatibility concerns about the superset thing. But let's wait until the series hits git@vger. Great work Duy! I see that you've used format_commit_message(), but there are some concerns about pretty-formats conflicting with f-e-r's format. We'll iron it out slowly, but I like the overall approach. Thanks. (Very sleep deprived at the moment; will review and collaborate after I wake up) -- 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 0/3] Towards a useable git-branch
On Fri, May 24, 2013 at 9:19 PM, Ramkumar Ramachandra artag...@gmail.com wrote: There is no need to use a hammer and coerce everything into an atom, or throw everything out the window and start from scratch to conform to pretty-formats perfectly. Let's extend the existing format to be _useful_ sensibly. Usefulness is one thing. Another is maintenance and in that regard I still think we should be able to remove -v and -vv code (not the functionality) with this topic. -- Duy -- 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 0/3] Towards a useable git-branch
Duy Nguyen wrote: Usefulness is one thing. Another is maintenance and in that regard I still think we should be able to remove -v and -vv code (not the functionality) with this topic. Why? The topic adds good functionality, doesn't break anything, doesn't paint us into any corner, and has users. Replacing -v and -vv can be done eventually, after we get alignment. -- 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 0/3] Towards a useable git-branch
Duy Nguyen pclo...@gmail.com writes: On Fri, May 24, 2013 at 9:19 PM, Ramkumar Ramachandra artag...@gmail.com wrote: There is no need to use a hammer and coerce everything into an atom, or throw everything out the window and start from scratch to conform to pretty-formats perfectly. Let's extend the existing format to be _useful_ sensibly. Usefulness is one thing. Another is maintenance and in that regard I still think we should be able to remove -v and -vv code (not the functionality) with this topic. Yes, the aim of the topic should be to make the machinery flexible enough so that we can lose -v/-vv implementation and replace them with calls to the new machinery with canned set of output format templates. By the way, I do not think useable git-branch is a good title, though. The expression has unnecessary venom in it, as if what it currently does is not usable. More flexible would be fine. I am having this feeling that we see more and more of this line of bad behaviours from some on the list recently to call something that is working in which they find itch using unnecessarily derogatory terms, and it makes people simply avoid any discussion that starts with such an attitude. Unnecessary negativity is not productive and it has to stop. -- 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 0/3] Towards a useable git-branch
Junio C Hamano wrote: I am having this feeling that we see more and more of this line of bad behaviours from some on the list recently to call something that is working in which they find itch using unnecessarily derogatory terms, and it makes people simply avoid any discussion that starts with such an attitude. Unnecessary negativity is not productive and it has to stop. My apologies. After all, we have all been using 'git branch' for so many years without complaining. I only noticed the itch recently: it's a burning itch that I want it to be fixed very badly (hence the series). If anything I intended to convey the importance of the patch to me personally, not about some general truth on the broken-ness of git-branch. Ofcourse I will take your suggestion and tone down, because I don't want the git community to feel bad about the software they're developing. -- 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 0/3] Towards a useable git-branch
Junio C Hamano wrote: Yes, the aim of the topic should be to make the machinery flexible enough so that we can lose -v/-vv implementation and replace them with calls to the new machinery with canned set of output format templates. Definitely. I don't want to keep my ugly alias around forever, and I certainly want more users to have easy access to this (configurable git-branch output formats). However, the series is not the theoretical exercise of prettifying the code underlying -v and -vv. Supporting -v and -vv is something we _have_ to do to preserve backward compatibility, and I would consider it a side-effect of the series rather than the aim of the topic. The aim of the topic is to get more useful output from git-branch. As long as the topic doesn't paint us into a corner after which it will be impossible to implement -v and -vv on top of the format, I think we're good. -- 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 0/3] Towards a useable git-branch
On Fri, May 24, 2013 at 9:19 PM, Ramkumar Ramachandra artag...@gmail.com wrote: So, while investigating alignment operators in pretty-formats, I found out that it's way too much effort and totally not worth it (atleast not immediately; we can add it later if we want). I just had an idea that might bring pretty stuff to for-each-ref with probably reasonable effort, making for-each-ref format a superset of pretty. But I need to clean up my backlog first. Give me a few days, I will show you something (or give up ;) -- Duy -- 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
