On Sun, May 19, 2013 at 6:11 PM, Ramkumar Ramachandra <artagnon@xxxxxxxxx> wrote: > Nguyễn Thái Ngọc Duy wrote: >> The purpose of this series is to make "for-each-ref --format" powerful >> enough to display what "branch -v" and "branch -vv" do so that we >> could get rid of those display code and use for-each-ref code instead. > > Damn, you beat me to it. I just introduced color, and was working on > alignment. See $gmane/224692. Hmm.. I missed that mail (or I wouldn't have worked on this already). Do you want to take over? >> - %(tracking[:upstream]) gives us the exact output that branch -v[v] >> does. %(upstream) does not include []. We can't change its >> semantics. > > There's already an atom called "upstream", and "upstream:short" works. > Why not introduce "upstream:diff" for "[ahead x, behind y]" and > "upstream:shortdiff" for "<>" (like in the prompt)? "branch -vv" shows [upstream: ahead x, behind y]. We need a syntax to cover that too. >> - %(color:...) is pretty much the same as %C family in pretty code. >> I haven't added code for %(color:foo) == %C(foo) yet. There's a >> potential ambiguity here: %C(red) == %Cred or %C(red)?? > > I'd vote for dropping %C<name> altogether and just go with %C(<name>). > Why do we need %(color:<name>) at all? pretty and for-each-ref format seem to be on the opposite: one is terse, one verbose. Unless you are going to introduce a lot of new specifiers (and in the worst case, bring all pretty specifiers over, unify underlying code), I think we should stick with %(xx) convention. >> - %(...:aligned) to do left aligning. I'm not entirely sure about >> this. We might be able to share code with %>, %< and %>< from >> pretty.c. But we need improvements there too because in >> for-each-ref case, we could calculate column width but %< would >> require the user to specify the width. > > Yeah, I think we should go with the %> and %< you introduced in > pretty.c. Yes, I want to be able to specify width. I still think we should follow %(...), e.g. %(left:N), %(right:N) as equivalent of %< and %>... >> Do people expect fancy layout with for-each-ref (and branch)? If so >> we might need to have %(align) or something instead of the simple >> left alignment case in %(...:aligned) > > Why should we deviate from the pretty case? What is different here? Laziness plays a big factor :) So again, you want to take over? ;) >> - We may need an equivalent of the space following % in pretty >> format. If the specifier produces something, then prepend a space, >> otherwise produce nothing. Do it like %C( tracking) vs >> %C(tracking)?? > > Yeah, sounds good. > >> You can try this after applying the series, which should give you the >> about close to 'branch -v'. %(tracking) coloring does not work though. > > Why doesn't %(tracking) coloring work? it uses builtin/branch.c:branch_use_color. Eventually fill_tracking_info() should be moved to for-each-ref.c and pass branch_use_color in as an argument. But for now, I just leave it broken. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html