pre-commit hook not run on conflict resolution during rebase
When a rebase stops because of a conflict, and I edit the file to resolve the conflict and say "git rebase --continue", then the pre-commit hook doesn't run at that point, which means that I can commit bad stuff which the pre-commit hook would normally not allow in. We were bitten by this a few times already. (In our case, the hook rejects code that hasn't been run through clang-format. That's easy to forget when resolving conflicts during a rebase.) >From glancing through the githooks manpage, I couldn't see any other hook that would help in this situation. Am I missing something? I guess the next best solution would be to also have a pre-push hook that performs the same checks again, just in case the bad code managed to get past the pre-commit hook for some reason or other. This feels very redundant, but I guess it would work well. Any other suggestions? -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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: pre-commit hook not run on conflict resolution during rebase
Junio C Hamano wrote: > li...@haller-berlin.de (Stefan Haller) writes: > > > I guess the next best solution would be to also have a pre-push hook > > that performs the same checks again, just in case the bad code managed > > to get past the pre-commit hook for some reason or other. This feels > > very redundant, but I guess it would work well. > > I'd say pre-receive would be the most sensible place to check things > like this. Yes, I totally agree, and we used to have this setup when we were still hosting our code in-house; with pre-receive doing the authorative checks, and pre-commit being optional as a convenience for developers, as you say. Now we have moved most of our code to github, and you can't have pre-receive hooks there, as far as I can tell. (I should have mentioned that, sorry.) To make up for that, we have put considerable effort into ensuring that everyone on the team has up-to-date hooks locally, by installing them automatically as part of our build system infrastructure. In that light, do you agree that a pre-push hook is the best we can do now to plug this hole? -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] gitk: Add a "Copy commit summary" command
Junio C Hamano wrote:
> Beat Bolli writes:
>
> > When referring to earlier commits in commit messages or other text, one
> > of the established formats is
> >
> > ("", )
> > ...
> > +proc copysummary {} {
> > +global rowmenuid commitinfo
> > +
> > +set id [string range $rowmenuid 0 7]
> > +set info $commitinfo($rowmenuid)
> > +set commit [lindex $info 0]
>
> 7 hexdigits is not always an appropriate value for all projects.
> The minimum necessary to guarantee uniqueness varies on project, and
> it is not a good idea to hardcode such a small value. Not-so-old
> Linux kernel history seems to use at least 12, for example.
>
> I believe that the "one of the established formats" comes from a
> "git one" alias I published somewhere long time ago, that did
> something like this:
>
> git show -s --abbrev=8 --pretty='format:%h (%s, %ai' "$@" |
> sed -e 's/ [012][0-9]:[0-5][0-9]:[0-5][0-9] [-+][0-9][0-9][0-9][0-9]$/)/'
>
> where the combination of --abbrev=8 and format:%h asks for a unique
> abbreviation that is at least 8 hexdigits long but can use more than
> 8 if it is not long enough to uniquely identify the given commit.
For the intended use case of this feature (referring to earlier commits
in commit messages), guaranteeing uniqueness isn't sufficiant either.
What is unique at the time of creating the commit might no longer be
unique a few years later.
So one strategy would be to add one or two digits to what %h returns, to
give some future leeway; or rely on the user to configure core.abbrev
appropriatly for their project; or just make the hard-coded value
configurable, as Hannes suggests.
FWIW, a discussion of this that I find useful can be found here:
<http://blog.cuviper.com/2013/11/10/how-short-can-git-abbreviate/>.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
--
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: Fast-forward able commit, otherwise fail
Junio C Hamano wrote: > Another thing to consider is that the proposed workflow would not > scale if your team becomes larger. Requiring each and every commit > on the trunk to be a merge commit, whose second parent (i.e. the tip > of the feature branch) fast-forwards to the first parent of the > merge (i.e. you require the feature to be up-to-date), would mean > that Alice and Bob collaborating on this project would end up > working like this: > > A:git pull --ff-only origin ;# starts working > A:git checkout -b topic-a > A:git commit; git commit; git commit > B:git pull --ff-only origin ;# starts working > B:git checkout -b topic-b > B:git commit; git commit > > A:git checkout master && git merge --ff-only --no-ff topic-a > A:git push origin ;# happy > > B:git checkout master && git merge --ff-only --no-ff topic-b > B:git push origin ;# fails! > > B:git fetch origin ;# starts recovering > B:git reset --hard origin/master > B:git merge --ff-only --no-ff topic-b ;# fails! > B:git rebase origin/master topic-b > B:git checkout master && git merge --ff-only --no-ff topic-b > B:git push origin ;# hopefully nobody pushed in the meantime > > The first push by Bob fails because his 'master', even though it is > a merge between the once-at-the-tip-of-public-master and topic-b > which was forked from that once-at-the-tip, it no longer fast-forwards > because Alice pushed her changes to the upstream. > > And it is not sufficient to redo the previous merge after fetching > the updated upstream, because your additional "feature branch must > be up-to-date" requirement is now broken for topic-b. Bob needs to > rebuild it on top of the latest, which includes what Alice pushed, > using rebase, do that merge again, and hope that nobody else pushed > to update the upstream in the meantime. As you have more people > working simultanously on more features, Bob will have to spend more > time doing steps between "starts recovering" and "hopefully nobody > pushed in the meantime", because the probability is higher that > somebody other than Alice has pushed while Bob is trying to recover. > > The time spend on recovery is not particularly productive, and this > workflow gives him a (wrong) incentive to do that recovery procedure > quickly to beat the other participants to become the first to push. I have read and re-read this thread a hundred times now, but I still don't understand why it makes much of a difference whether or not Bob rebases his branch onto master before pushing his merge. In both cases, Alice and Bob have this race as to whose push succeeds, and in both cases you end up with merge commits on master that are not well tested. First of all, let me say that at my company we do use the workflow that David suggests; we rebase topic branches onto master before merging them (with --no-ff), and we like the resulting shape of the history. Even the more experienced git users like it for its simplicity; it simply saves us time and mental energy when digging through the history. Second, we did indeed run into the scalability problems that you describe [*1*]. However, we ran into this way before starting to require the rebase-before-merge; in my experience, rebasing or not makes no difference here. After all, the resulting tree state of the merge commit is identical in both cases; it's just the individual commits on the merged topic branch that have not been tested in the rebased case. But if the merge commit is green, it is pretty unlikely in my experience that one of the individual commits is not. It's theoretically possible of course, just very, very unlikely. So what am I missing? [Footnote] *1* These problems were so annoying for us that we invented technical measures to solve them. We now have a web interface where developers can grab a lock on the repo, or put themselves into a queue for the lock when it's taken. There's a push hook that only allows pushing when you hold the lock. This solves it nicely, because once you have the lock, you can take all the time you need to make sure your merge compiles, and run the test suite locally. -- 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
Limitiations of git rebase --preserve-merges --interactive
The BUGS section of the git-rebase manpage says that editing or rewording commits "should work fine", but attempts to reorder commits usually don't do what you want. I'd like to know more about what does or doesn't work. For example, will squashing commits work? (I.e. using the "fixup" or "squash" commands in the todo sheet.) Will dropping commits work? Does it make sense to insert "exec" commands, or will they run at arbitrary times? -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Limitiations of git rebase --preserve-merges --interactive
Stefan Beller wrote:
> On Thu, Sep 22, 2016 at 12:48 PM, Kevin Daudt wrote:
> > On Thu, Sep 22, 2016 at 07:33:11PM +, Anatoly Borodin wrote:
> >> Hi Stefan,
> >>
> >> this section was added to the manual in the commit
> >> cddb42d2c58a9de9b2b5ef68817778e7afaace3e by "Jonathan Nieder"
> >> 6 years ago. Maybe he remembers better?
> >>
> >
> > Just to make it clear, this section explicitly talks about 'bugs' with
> > preserve-merges and interactive rebase. Without the --preserve-merges
> > option, those operations works as expected.
> >
> > The reason, as that section explains, is that it's not possible to store
> > the merge structure in the flat todo list. I assume this means git
> > internally remembers where the merge commit was, and then restores it
> > while rebasing.
> >
> > Changing the order, or dropping commits might then give unexpected
> > results.
> >
>
> The commit message may help as well:
>
> rebase -i -p: document shortcomings
>
> The rebase --preserve-merges facility presents a list of commits
> in its instruction sheet and uses a separate table to keep
> track of their parents. Unfortunately, in practice this means
> that with -p after most attempts to rearrange patches, some
> commits have the "wrong" parent and the resulting history is
> rarely what the caller expected.
>
> Yes, it would be nice to fix that. But first, add a warning to the
> manual to help the uninitiated understand what is going on.
Thanks, but all of this still talks about the issues in very generic
terms ("most attempts to rearrange patches"). I'm interested in more
details as to exactly what kind of attempts do or don't work. In
particular, I'm interested in fixup/squash commands (without reordering
anything else), or dropping (non-merge) commits.
I could of course experiment with these and try to find out myself, but
I was hoping someone would just know the answer off the top of their
head, saving me some time.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
Re: Limitiations of git rebase --preserve-merges --interactive
Anatoly Borodin wrote: > PS There are also some pieces of "what should work" in these tests: > > t/t3409-rebase-preserve-merges.sh* > t/t3410-rebase-preserve-dropped-merges.sh* > t/t3411-rebase-preserve-around-merges.sh* > t/t3414-rebase-preserve-onto.sh* Thanks, this is interesting; I'm having trouble understanding the tests though. Some of them use rebase -p -i, but I don't understand why they use -i, or why that even works in a test (i.e. why it doesn't open an editor). In one test I saw "GIT_EDITOR=: git rebase -i -p", which I guess means "use the initially given todo sheet unchanged". I don't see any tests that do an interactive rebase and actually change the todo list. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Limitiations of git rebase --preserve-merges --interactive
Junio C Hamano wrote: > li...@haller-berlin.de (Stefan Haller) writes: > > > Thanks, this is interesting; I'm having trouble understanding the tests > > though. Some of them use rebase -p -i, but I don't understand why they > > use -i, or why that even works in a test (i.e. why it doesn't open an > > editor). > > Upon starting up, tests dot-source t/test-lib.sh file and it > unsets most of GIT_* environment variables to obtain a stable > testing environment that is not affected by things that testers > may have in their environment. > > There is EDITOR=: in t/test-lib.sh, which was added in 2006 before > GIT_EDITOR was invented. That is the one in effect for git > subcommands that usually interacts with editors during the test, > unless specific tests further override it with test_set_editor > helper. Thanks for the explanation. So this explains why -i works at all here; it doesn't explain why -i is used in these tests. Unless I'm missing something, they should all work with just -p. And I don't see any tests that do rebase -p -i and actually do something interesting with the -i part. So my original question still remains. :-) -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Optimizing writes to unchanged files during merges?
Lars Schneider wrote: > An engineer works on a task branch and runs incremental builds — all > is good. The engineer switches to another branch to review another > engineer's work. This other branch changes a low-level header file, > but no rebuild is triggered. The engineer switches back to the previous > task branch. At this point, the incremental build will rebuild > everything, as the compiler thinks that the low-level header file has > been changed (because the mtime is different). If you are using C or C++, look into ccache. We're using it to work around this problem, and it's a near-perfect solution, I'd say.
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Jeff King wrote:
> On Sun, Apr 09, 2017 at 10:38:42AM +0200, Stefan Haller wrote:
>
> > I think it's wrong to think about these leases as something that you
> > take before you start a rewindy operation. That's the wrong time to take
> > the lease; by that time, the remote tracking branch may already contain
> > new things that you haven't seen yet, so using that as a lease at that
> > time will overwrite those things later. You have to take the lease at a
> > time where you know that your local branch and the remote tracking
> > branch are up to date with each other, which is after pull and push. And
> > if you do that, there's no multiple-operation ambiguity to deal with at
> > all.
>
> OK. I was assuming that you'd have just integrated before starting such
> a rebase, but I guess that doesn't have to be the case.
>
> I agree that probably makes the multiple-operation stuff go away, which
> is nice. It does raise the question of when the integration point
> happens, and how we handle alternate paths through which commits may
> land in a local branch (e.g., if both you and upstream do a ff-merge of
> a particular branch).
Are you talking about the case where the user doesn't say git pull, but
instead says "git fetch && git merge --ff @{u}"? Just so that I
understand the concern.
> I think that would probably just end up with extra
> failures though (so erring on the side of caution).
Yes, and I think this is a very important decision in general.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Jacob Keller wrote: > On Sun, Apr 9, 2017 at 4:00 AM, Stefan Haller wrote: > > > Maybe I wasn't clear enough about that in my proposal, but I propose to > > always store the commit hash of the remote tracking branch as a new > > lease after push and pull, not the local branch. This way it works > > nicely with pull --rebase and a branch that has extra local commits. > > Oh right. The main thing it doesn't give is that this doesn't enforce > that your local branch *has* to have at one point prior to the push > matched the remote branch that you're overwriting, which would be even > more evidence that your changes are what you expect and aren't > deleting anything unexpectedly. I don't think it's a necessary requirement to enforce that the local branch has to *match* the remote branch (i.e. point at the exact same commit) prior to beginning a rewindy operation. All that matters is that the local branch is what I called "up to date" with the remote branch in earlier messages; a more precise way of saying this is that the remote branch must either be the same as the local branch, or be reachable from the local branch (in other words, local branch "contains" the remote branch but has more commits on top). If we take the lease after every push and every successful pull, then this should be guaranteed, as far as I can see. (The "successful" here is a bit problematic, because if the pull fails with conflicts, then we need to wait until the next commit (if it was a merge) or the next "rebase --continue" to be able to tell if it was successful. More on that in a separate mail later.) -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Ævar Arnfjör? Bjarmason wrote: > Does this proposal require that all the things that can update a ref > be hooked to maintain these lease values? It is true that the proposal relies on people using git push and git pull, not some lower level approximation such as git fetch + git update-ref. Whether that's a valid assumption, I'm not sure yet. It does mean that there are GUI tools that will break the feature; e.g. SmartGit does use fetch + update-ref when you tell it to pull. In general, I'm not too concerned with my proposal not supporting certain edge-cases such as the one you described later in your mail. I think it's fine if you have to fall back to using --force-with-lease with explicit arguments in these cases. The suggestion is really only to make the common case easier, which (for me) is working with a tracking branch, and using push and pull with no arguments. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Jeff King wrote:
> On Tue, Apr 11, 2017 at 02:37:27PM +0200, Stefan Haller wrote:
>
> > Are you talking about the case where the user doesn't say git pull, but
> > instead says "git fetch && git merge --ff @{u}"? Just so that I
> > understand the concern.
>
> Yes, that (which is the main way that I merge changes).
OK; in my proposal I already mentioned that a few other commands besides
push and pull may have to update the lease; examples I mentioned were
git rebase @{u}
git reset @{u}
and you add "git merge --ff @{u}" to that list now. There might be
others that we can add to make the feature work better. (But this could
happen incrementally later, as we learn about more use cases.)
> But also what happens with:
>
> git merge origin/other-branch
> git rebase origin/master
>
> I think we only care when origin/master has independently merged
> other-branch, too. And even though we have taken its commits into
> account, we would fail (because "both sides did the same thing" is
> really out of scope for the concept of a lease). So that's OK.
I think there's nothing special to consider here; "git rebase
origin/master" updates the lease (to origin/master), period. It doesn't
matter whether origin/other-branch was merged before, or whether or not
it was independently merged to origin/master too, or whether our local
branch has cherry-picked all the commits of other-branch instead of
merging it, or whatever. In all these cases, the local branch is "up to
date" with origin/master after the rebase, so it's ok to update the
lease at that point.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Stefan Haller wrote:
> Then, every command that either integrates the remote tracking branch
> into the local branch, or updates the remote tracking branch to the
> local branch, will update the value of the "lease" entry. The most
> obvious ones are "git pull" and "git push", or course;
I thought a bit more about what this means, concretely. The problem is
that only a *successful* pull must update the lease; an unsuccessful
pull must leave it alone. Pull is either fetch+merge or fetch+rebase,
and if the merge or rebase fails with conflicts, then we can only tell
much later whether the pull was successful; in the case of merge only
after the next commit (success) or after git merge --abort (failure),
and in the case of rebase after the next rebase --continue (success), or
rebase --abort (failure).
To implement this, git pull could set the value "branch.*.pending-lease"
after fetch was successful (but leave "branch.*.lease" alone); then git
merge and git rebase could look for that value, and if successful, set
branch.*.lease to the value of branch.*.pending-lease, and delete
pending-lease. If unsuccessful, they'd just delete the pending-lease
entry. Other command may also have to delete the pending-lease entry,
e.g. git reset.
I do realize that this is a lot of complexity, and it has the potential
of missing some cases. However, this complexity is also the reason why I
can't build my own wrappers around pull/push to implement the feature
outside of git; alias mypull='git pull && git tag -f lease @{u}' just
doesn't cut it.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Junio C Hamano wrote: > On Tue, Apr 11, 2017 at 8:33 AM, Jacob Keller wrote: > > > > If you're already copying sha1s around you could use those as the > > --force-with-lease=branch:, no? > > > > That's better guarantee than just using --force-with-lease alone. > > Absolutely. That was the _only_ way the feature was originally designed > to be used sensibly. We really shouldn't have added that "lazy" option [...] > > Perhaps we should deprecate that "lazy" feature and remove it over > time, making sure that everybody feeds the explicit commit object name > that was recorded by the user somewhere (e.g. the approach to tag the > commit to record the expected remote tip, which Peff illustrated). I agree that this is better than giving the user a false sense of security. The problem is that manually recording the lease is painful. Like I illustrated, the assumption that you can "simply" do this: (... my working copy is in some ramdom state) (... now I decide I want to rewrite history) $ git tag lease origin/master $ git rebase -i $ git push --force-with-lease=master:lease doesn't hold, because by the time you decide to rewrite the history it's already too late. To solve this, I'd have to record the lease every time I pull or push; this is a lot of work, and easy to forget and error-prone. Hence my suggestion to have git do this automatically. I'd be interested in your thoughts about that proposal, Junio; you didn't say anything about that at all yet. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: [PATCH] push: disable lazy --force-with-lease by default
Junio C Hamano wrote: > It turns out that some people use third-party tools that fetch from > remote and update the remote-tracking branches behind users' back, > defeating the safety relying on the stability of the remote-tracking > branches. Third-party tools are not the only problem. They may make the problem more likely to occur, but it can also happen without them. (See below.) > Let's disable the form that relies on the stability of remote-tracking > branches by default, and allow users who _know_ their remote-tracking > branches are stable to enable it with a configuration variable. I'm wondering if people who claim they know they are safe really do. Elsewhere in the other thread somebody said "I only ever explicitly fetch, so I know I'm safe". Are you sure? Consider this example: $ git checkout the-branch-i-am-collaborating-on-with-my-collegue $ git pull # make sure I have their latest work $ git rebase -i ... # do some history rewriting # OK, so as we need to force-push anyway, let's take the opportunity and # rebase onto the latest master: $ git fetch # get latest master $ git rebase origin/master $ git push --force-with-lease This is a very common thing to do at my workplace. And it's unsafe, because the git fetch may move the remote-tracking branch of the branch I'm working on. To make this safe, I guess you'd have to replace "git fetch" with something like $ git fetch refs/heads/master:refs/remotes/origin/master Personally I have never used this form of fetch myself, and I'd be surprised if any of my coworkers even know it exists. So know you could decide that _any_ fetch is unsafe, and never use it; only use git pull. You are still not safe: $ git checkout the-branch-i-am-collaborating-on-with-my-collegue $ git pull $ git rebase -i # Now another collegue walks in and asks me to look at the regression # they just introduced on some other branch, so I do $ git checkout that-other-branch $ git pull $ $ # go back to what I was doing: $ git checkout the-branch-i-am-collaborating-on-with-my-collegue $ git push --force-with-lease Again, the git pull may have moved the remote-tracking branch of the branch that I want to force-push. Again, it could be solved by given an explicit refspec to git pull, but few people ever do this in my experience, and I certainly never want to. What I'm getting at is that there's a lot of things that you have to remember to not do in order to make --force-with-lease without parameter a useful tool. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Matt McCutchen wrote: > When I'm rewriting history, "git push --force-with-lease" is a nice > safeguard compared to "git push --force", but it still assumes the > remote-tracking ref gives the old state the user wants to overwrite. > Tools that do an implicit fetch, assuming it to be a safe operation, > may break this assumption. In the worst case, Visual Studio Code does > an automatic fetch every 3 minutes by default [1], making > --force-with-lease pretty much reduce to --force. That's a big problem, but even without such tools, I find --force-with-lease without an argument to be pretty limited in usefulness. I like to type "git fetch" myself regularly, just to see what's new upstream before integrating it; this already breaks it. But even avoiding "git fetch" doesn't help if you are working on more than one branch at a time, because doing "git pull" on one branch will do an implicit "git fetch" on the other. I like the idea of git maintaining a separate "last integrated" commit for each branch, I think this could solve it in a nice way. I'm probably not qualified enough to work on this myself though, but I'm happy to give input if someone else wants to. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Matt McCutchen wrote:
> When I'm rewriting history, "git push --force-with-lease" is a nice
> safeguard compared to "git push --force", but it still assumes the
> remote-tracking ref gives the old state the user wants to overwrite.
> Tools that do an implicit fetch, assuming it to be a safe operation,
> may break this assumption. In the worst case, Visual Studio Code does
> an automatic fetch every 3 minutes by default [1], making
> --force-with-lease pretty much reduce to --force.
>
> For a safer workflow, "git push" would check against a separate "old"
> ref that isn't updated by "git fetch", but is updated by "git push" the
> same way the remote-tracking ref is and maybe also by commands that
> update the local branch to take into account remote changes (I'm not
> sure what reasonable scenarios there are, if any).
Here's a rough proposal for how I would imagine this to work.
For every local branch that has a remote tracking branch, git maintains
a new config entry branch.*.integrated, which records the sha1 of the
last upstream commit that was integrated into the local branch.
When --force-with-lease is used without an argument, it will use the
values of "branch.*.remote:branch.*.integrated" as an argument. If
either doesn't exist, the push fails (this is essential).
Initially the "integrated" entry is created at the same time that
branch.*.merge is, i.e. with commits like "git checkout -b
name-of-remote-branch", or "git branch --set-upstream-to" and the like,
and it will be set to the sha1 that the tip of the remote tracking
branch has at that time.
Then, every command that either integrates the remote tracking branch
into the local branch, or updates the remote tracking branch to the
local branch, will update the value of the "integrated" entry. The most
obvious ones are "git pull" and "git push", or course; others that may
have to be supported are "git rebase @{u}", "git rebase --onto @{u}",
"git reset @{u}", and probably others. The nice thing about these is
that initially they don't have to be supported for the feature to still
be useful. After using one of them, push --force-with-lease will fail,
and the user can then investigate the situation and either use push -f
or manually update branch.*.integrated when they have convinced
themselves that everything is fine.
I find it essential that --force-with-lease might fail erroneously, but
never succeed erroneously, and I think this proposal would guarantee
that.
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Jeff King wrote: > I think Matt's point is just that the default, to use origin/branch, is > unsafe. It's convenient when you don't have extra fetches, but that > convenience may not be worth the potential surprise. I don't think "surprise" is the right word here. The point of --force-with-lease is to provide a guarantee, so if you can't trust the guarantee, it makes the feature rather pointless. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Ævar Arnfjör? Bjarmason wrote: > On Sat, Apr 8, 2017 at 5:03 PM, Stefan Haller wrote: > > > Here's a rough proposal for how I would imagine this to work. > > > > For every local branch that has a remote tracking branch, git maintains > > a new config entry branch.*.integrated, which records the sha1 of the > > last upstream commit that was integrated into the local branch. > > Can you elaborate on what "integrate" means in this context? > > In some ways the entire point of this feature is that you're trying to > push over history that you don't want to integrate. > > I.e. you're trying to force push your unrelated X over remote history > A, but not more recent arbitrary history B based on A which someone > may have just pushed. > > I'm having a hard time imagining how anything merge/rebase/whatever > would place in branch.*.integrated wouldn't just be a roundabout way > of recording the info we now record via the tracking branch, or in > cases where that's auto-updated for some reason having another > tracking branch as my "[PATCH] push: document & test > --force-with-lease with multiple remotes" suggests. It doesn't matter whether the history you are overwriting is arbitrary, or whether the new history you are pushing is related or unrelated to what you are overwriting. What matters is whether you are aware of what you are overwriting. I want to record all cases where the local branch is brought up to date with the tracking branch (or vice versa), i.e. mostly push and pull, because I know that after pushing or pulling, my local branch is up to date (in some way) with the tracking branch. If I then rewrite the local branch, I know it is safe to push it *if* the branch on the remote is still in the same state as what I recorded for last push or pull. If the tracking branch is updated by fetch though, then my local branch is not brought up to date with the remote branch, so I may be overwriting stuff that appeared on the remote without me being aware of it. It may well be that there are better names then "integrate"; suggestions welcome. Your suggestion to use a second remote doesn't seem like a satisfactory solution to me, firstly because it's extra work and complexity for the user, and second because it doesn't solve the problem of working with more than one local branch (pulling one branch amounts to a fetch for the other). -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Jacob Keller wrote: > What if we added a separate command something like: > > git create-lease > > which you're expected to run at the start of a rewind-y operation and > it creates a tag (or some other ref like a tag but in a different > namespace) which is used by force-with-lease? The problem with this is that it doesn't help to use "git create-lease" right before you start your rewind-y operation, because by that time you may already have fetched. You'd have to use "git create-lease" right after you pull or push. But at the time I pull I don't know yet whether I will later want to rewrite the branch, so to be sure I have to do this every time I pull or push, and then I'd prefer git to do it for me. > However, I think using origin/master works fine as long as you don't > auto-fetch. > > If you're doing it right, you can handle origin/master updates by > checking that your rewind-y stuff is correct for the new origin/master > RIGHT before you push. I'm not sure I understand what you mean by "checking that your rewind-y stuff is correct for the new origin/master"; does that mean manually inspecting origin/master to convince youself that you are not overwriting something new? If so, I don't think this is acceptable. It is probably ok to work this way if the other party only pushed commits on top; it's reasonable to expect that you will recognize new commits as ones that you haven't seen before. But what if the other party has rewritten the branch and squashed improvements into commits in the middle of it? The head commit will then look the same as before, and the only way to tell whether you are overwriting something new is by comparing the old and new hashes. So then we're back at having to remember what the old hash was. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Jeff King wrote: > > It might be possible to generate these lease tags prior to operations > > which modify history and then maybe having a way to list them so you > > can select which one you meant when you try to use force-with-lease.. > > So yeah, I think that is the more interesting direction. I hadn't > considered resolving the multiple-operation ambiguity at push time. But > I guess it would be something like "you did a rebase on sha1 X at time > T, and then one on Y at time T+N", and you pick which one you're > expecting. I think it's wrong to think about these leases as something that you take before you start a rewindy operation. That's the wrong time to take the lease; by that time, the remote tracking branch may already contain new things that you haven't seen yet, so using that as a lease at that time will overwrite those things later. You have to take the lease at a time where you know that your local branch and the remote tracking branch are up to date with each other, which is after pull and push. And if you do that, there's no multiple-operation ambiguity to deal with at all. > And I think that may be converging on the "integrate" refs that Stefan is > talking about elsewhere (or some isomorphism of it). Does it make things clearer if we don't use the term "integrate", but call the config value in my proposal simply "branch.*.lease"? -- Stefan Haller Ableton http://www.ableton.com/
Re: Tools that do an automatic fetch defeat "git push --force-with-lease"
Jacob Keller wrote: > Agreed. You "take" a lease whenever you push to the remote or when you > pull from the remote and when you pull into the branch. It should > store something that tracks both the branch and remote branch together > so that you can generalize it to multiple remotes. I don't see why it has to support multiple remotes (but then I don't have much experience with workflows involving multiple remotes, so I may well be missing something). A local branch can only have one remote tracking branch on one remote, and in my view --force-with-lease without arguments works with that remote tracking branch only. Is this view too simple? > It doesn't necessarily track perfectly with a branch that contains > extra work such as when doing pull --rebase, but maybe you have an > idea about that? Maybe I wasn't clear enough about that in my proposal, but I propose to always store the commit hash of the remote tracking branch as a new lease after push and pull, not the local branch. This way it works nicely with pull --rebase and a branch that has extra local commits. -- Stefan Haller Ableton http://www.ableton.com/
Re: [PATCH] gitk: different color for boundary commits
Stefan Dotterweich wrote: > When using filters, the commit list shows not only commits matching > the filter criteria, but also boundary commits. When going through a > list of say, all commits changing the variable `foo`, often half of > the displayed commits are boundary commits. In this case the boundary > commits are of little interest. > > However, there is no way to hide them or quickly distinguish them from > the actual commits. Boundary commits can be identified by the white > color inside the circle, but that is not easily recognisable. On each > line you have to look at the circle color to identify the commit > type. This makes it hard to just quickly skim a list of commits, > especially when looking at dates and authors which are further to the > right. > > Therefore, to make boundary commits easier to recognise, display their > text in a different color. I would like to go one step further and not show the boundary commits at all. Why do we need them again? (I think this has been discussed before, but I can't find it right now. The only reference I could find is this thread: <https://public-inbox.org/git/571f6852.1070...@qt.io/T/#u>, which doesn't explain *why* gitk shows the boundary commits in the first place.) In my opinion, when saying "gitk --author=foo", the list of commits in the top pane should look the same as the ouput of "git log --oneline --author=foo". -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/
Re: `git stash pop` UX Problem
Junio C Hamano wrote: > Stephen Leake writes: > > >> Dropping the stash on a "git add" operation would be really, really > >> weird... > > > > Why? That is when the merge conflicts are resolved, which is what > > logically indicates that the stash is no longer needed,... > > Not necessarily. Imagine a case where you used stash to quickly > save away a tangled mess that was not ready for a logically single > commit and now you are in the process of creating the first commit > by applying it piece-by-piece to create multiple resulting ones. > After you commit the result, you would still want to keep the parts > of that stashed change you did not include in the first commit so > that you can go back, no? > > You may run "git add", but that does not say anything about what you > are going to use the rest of the stash for. Not even "git commit" > may be a good enough sign. But we are only talking about the situation where you typed "git stash pop", and this resulted in a merge conflict. Your intention was clearly to drop the stash, it just wasn't dropped because of the conflict. Dropping it automatically once the conflict is resolved would be nice. I know it happened to me too that I forgot to drop a stash after resolving conflicts, so I'd appreciate a feature that somehow does this automatically for me. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] gitk: Enable mouse horizontal scrolling in diff pane
Gabriele Mazzotta wrote: > Currently it's required to hold Shift and scroll up and down to move > horizontally. Listen to Button-6 and Button-7 events too to make > horizontal scrolling handier with touchpads and some mice. Hm, on my Macbook the diff pane already scrolls in all four directions with two-finger scrolling on the trackpad. (Without your patch, I mean.) In my copy of gitk I did the opposite for the commit list though; I can't stand it that I accidentally scroll left or right with the trackpad, so I commented out some code to restrict it to only vertical scrolling. I never posted this patch because I bet many people like the current behaviour. Just so you know that such a change might be controversial. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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: bug: git merge --no-commit loses track of file modes in the index
Joey Hess wrote: > If git merge --no-commit is used to merge a commit adding a > file with an unusual mode -- specifically a symlink which has "mode" 12, > it fails to stage the right mode into the index. > > This only happens when core.symlinks=false. I noticed it on FAT, but > have managed to reproduce it on ext4. This sounds familiar; I wonder if it is related to the problem that git can lose the executable bit when core.filemode is false. <http://thread.gmane.org/gmane.comp.version-control.git/159716> I had planned to look into fixing this for years now, as we still run into it once in a while, and it's pretty annoying; but I still didn't get around to it yet. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] gitk: make pointer selection visible in highlighted lines
Paul Mackerras wrote: > On Thu, Nov 28, 2013 at 11:20:18PM +0200, Max Kirillov wrote: > > Custom tags have higher priority than `sel`, and when they define their > > own background, it makes selection invisible. Especially inconvenient > > for `filesep` (to select filenames), but also affects other tags. > > Use `tag raise` to fix `sel`'s priority. > > > > Also change `omark` tag handling, so that it is created once, together > > with others, and then only removed from text rather than deleted. Then > > it will not get larger priority than the `sel` tag. > > This conflicts with the recent change to highlight the current search > hit in orange (c46149, "gitk: Highlight current search hit in > orange"). With the selection being the highest-priority tag, the > current search hit gets shown in grey instead. I think I prefer the > orange. I agree though that if the user explicitly selects part of > the text using the mouse, the selection highlight should be the > highest priority in that case. Maybe the solution is to not select > the search hit automatically? I don't think that not selecting the search hint is an option: the selection is used to keep track of where to search next. Can't we just raise the currentsearchhit tag above the sel tag? -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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: [wish] Revert changes in git gui
Christoph Grüninger wrote: > While browsing the changes, it is very easy to add (or remove) lines or > hunks for commit via the context menu. I would like to revert the > changes of a line or a hunk in a similar way. I have often white-space > or formatting changes I don't want to commit but want them reverted > immediately. I would like to have this feature too. In the meantime, my workaround is to stage the whole file, unstage the hunk(s) or lines that I want to revert, and then choose "Revert Changes" from the Commit menu. A little more awkward than I would like, but it works well (except when you already took some time to selectively stage hunks or lines for the next commit). So yes, a context menu for this would be very welcome. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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: Show patch in gitk --first-parent ?
Karl Wiberg wrote: > With "gitk --first-parent", I get a graph that only follows the first > parent of every merge---perfect if you always merge topic branches > into the main branch. However, the diff shown is still the normal > merge diff, and not the diff between the new tree and the first > parent's tree---is it possible to make it do that instead? I once posted a patch that adds a "First parent" checkbox to gitk's window: <http://comments.gmane.org/gmane.comp.version-control.git/160920> The patch no longer applies today, but I can send an updated version that does, if there's interest. The topic didn't go anywhere for two reasons: 1) There's the confusion about history traversal option (the existing --first-parent command-line option) versus diff option (the new check box); they have similar names, but control different things (and it should be possible to control these independently). 2) Space is short in the diff pane; you need to make the window rather wide to see them all. I didn't have the energy to drive these to a resolution back then; if you could do that, it would be great. Personally I'm using my own gitk with my patch applied, and I do use the "First parent" checkbox rather often. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] gitk: Add "First parent" checkbox
Sometimes it's desirable to see what changes were introduced by a
merge commit, rather than how conflicts were resolved. This adds
a checkbox which, when turned on, makes gitk show the equivalent
of "git show --first-parent " for merge commits.
Signed-off-by: Stefan Haller
---
This is the same patch as the one I sent in
<http://comments.gmane.org/gmane.comp.version-control.git/160920>, with
the same issues discussed in that thread. I just brought it up to date
with current master.
gitk | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index 5cd00d8..3466054 100755
--- a/gitk
+++ b/gitk
@@ -2336,6 +2336,10 @@ proc makewindow {} {
pack .bleft.mid.worddiff -side left -padx 5
}
+${NS}::checkbutton .bleft.mid.firstparent -text [mc "First parent"] \
+ -command changefirstparent -variable firstparent
+pack .bleft.mid.firstparent -side left -padx 5
+
set ctext .bleft.bottom.ctext
text $ctext -background $bgcolor -foreground $fgcolor \
-state disabled -font textfont \
@@ -7080,6 +7084,7 @@ proc selectline {l isnew {desired_loc {}}} {
global cmitmode showneartags allcommits
global targetrow targetid lastscrollrows
global autoselect autosellen jump_to_here
+global firstparent
catch {unset pending_select}
$canv delete hover
@@ -7221,7 +7226,7 @@ proc selectline {l isnew {desired_loc {}}} {
init_flist [mc "Comments"]
if {$cmitmode eq "tree"} {
gettree $id
-} elseif {[llength $olds] <= 1} {
+} elseif {[llength $olds] <= 1 || $firstparent} {
startdiff $id
} else {
mergediff $id
@@ -7624,7 +7629,7 @@ proc diffcmd {ids flags} {
proc gettreediffs {ids} {
global treediff treepending limitdiffs vfilelimit curview
-set cmd [diffcmd $ids {--no-commit-id}]
+set cmd [diffcmd $ids {--no-commit-id -m --first-parent}]
if {$limitdiffs && $vfilelimit($curview) ne {}} {
set cmd [concat $cmd -- $vfilelimit($curview)]
}
@@ -7710,12 +7715,20 @@ proc changeworddiff {name ix op} {
reselectline
}
+proc changefirstparent {} {
+global treediffs
+catch {unset treediffs}
+
+reselectline
+}
+
proc getblobdiffs {ids} {
global blobdifffd diffids env
global diffinhdr treediffs
global diffcontext
global ignorespace
global worddiff
+global firstparent
global limitdiffs vfilelimit curview
global diffencoding targetline diffnparents
global git_version currdiffsubmod
@@ -7728,13 +7741,18 @@ proc getblobdiffs {ids} {
if {[package vcompare $git_version "1.6.6"] >= 0} {
set submodule "--submodule"
}
-set cmd [diffcmd $ids "-p $textconv $submodule -C --cc --no-commit-id
-U$diffcontext"]
+set cmd [diffcmd $ids "-p $textconv $submodule -C --no-commit-id
-U$diffcontext"]
if {$ignorespace} {
append cmd " -w"
}
if {$worddiff ne [mc "Line diff"]} {
append cmd " --word-diff=porcelain"
}
+if {$firstparent} {
+ append cmd " -m --first-parent"
+} else {
+ append cmd " --cc"
+}
if {$limitdiffs && $vfilelimit($curview) ne {}} {
set cmd [concat $cmd -- $vfilelimit($curview)]
}
@@ -11865,6 +11883,7 @@ set diffcontext 3
set mergecolors {red blue green purple brown "#009090" magenta "#808000"
"#009000" "#ff0080" cyan "#b07070" "#70b0f0" "#70f0b0" "#f0b070" "#ff70b0"}
set ignorespace 0
set worddiff ""
+set firstparent 0
set markbgcolor "#e0e0ff"
set headbgcolor green
--
1.8.3.2.747.g15edaa9
--
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: "Show origin of this line" triggered programatically
The "Show origin of this line" context menu command on a line in a patch
in gitk is by far my most frequently used way of navigating back in
history. It's so much faster than git gui blame.
Now, I wish it were available right from my text editor, showing me the
origin of the line under the cursor when working with code.
In fact, it is; I have been using this feature in a roughly hacked way
for a couple of months now, and I don't want to miss it any more. Most
of my co-workers are jealous when they see it.
I'd like to get it into gitk for real now, and I need some guidance on
the best way to implement it.
I can see two ways:
1) Add an option like '--show-origin=foo.cpp:25'. This would run
"git blame" on the given line in the working copy, and highlight the
result.
2) Add an option like '--select-file=foo.cpp:25'. This would be used in
combination with the existing --select-commit option to simply
highlight a given line in the patch (so the caller needs to do the
initial git blame himself).
There are pros and cons to both. 1) is certainly easier to use from a
text editor scripter's point of view (a simple one-liner in most cases),
whereas 2) probably needs a glue script to run the initial git blame. On
the other hand, that glue script could also take care of restricting the
history to a week or so forward from the target commit, which makes gitk
load much faster. Also, 2) could be used from git gui blame's "Show
history context" command, which currently only selects the commit but
can't highlight the line.
So I think I'd lean towards option 2). Below is the prototype that I
have been using; not sure how buggy it is. If we go this route, one
question would be whether we want to ship the glue script too, and how.
Any opinions?
-Stefan
>From 80acb168ef13a55521e9b821450800450660769d Mon Sep 17 00:00:00 2001
From: Stefan Haller
Date: Thu, 18 Jul 2013 18:55:11 +0200
Subject: [PATCH] gitk: Add options --select-file and --select-line
These can be used in combination with --select-commit to jump to a given
line in a patch. This can be useful for scripting your text editor to bring
up gitk showing you the place in the history that last modified the line
you are on.
For example, given this little glue script, saved as "show_line_in_gitk.rb"
somewhere in your path:
#!/usr/bin/env ruby
if ARGV.length != 2
puts "Usage: #{$0} "
exit 1
end
file, line = ARGV
blame_output = `git blame -p -L#{line},+1 "#{file}"`
exit 1 if $?.exitstatus != 0
blame_output_lines = blame_output.split("\n")
commit, line = blame_output_lines[0].split
file = blame_output_lines.grep(/^filename /)[0][9..-1]
date = blame_output_lines.grep(/^committer-time /)[0][15..-1]
datestr = Time.at(date.to_i + 60 * 60 * 24 * 7).to_s
system "gitk --before='#{datestr}' \
--select-commit=#{commit} \
--select-file='#{file}' \
--select-line=#{line} &"
you could add this function to your .vimrc:
function! ShowLineInGitk()
execute "!show_line_in_gitk.rb " . expand("%p") . " " . line(".")
endfunction
and map it to a key.
---
gitk | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index 5cd00d8..318a484 100755
--- a/gitk
+++ b/gitk
@@ -464,12 +464,17 @@ proc stop_rev_list {view} {
}
proc reset_pending_select {selid} {
-global pending_select mainheadid selectheadid
+global pending_select pending_select_file pending_select_line
+global mainheadid selectheadid selectfile select_line
if {$selid ne {}} {
set pending_select $selid
} elseif {$selectheadid ne {}} {
set pending_select $selectheadid
+ if {$selectfile ne {}} {
+ set pending_select_file $selectfile
+ set pending_select_line $select_line
+ }
} else {
set pending_select $mainheadid
}
@@ -1597,6 +1602,17 @@ proc getcommitlines {fd inst view updating} {
return 2
}
+proc select_pending_line {} {
+global pending_select pending_select_file pending_select_line
+
+if {[info exists pending_select_file]} {
+ selectline [rowofcommit $pending_select] 1 \
+ [list $pending_select_file $pending_select_line]
+} else {
+ selectline [rowofcommit $pending_select] 1
+}
+}
+
proc chewcommits {} {
global curview hlview viewcomplete
global pending_select
@@ -1611,7 +1627,7 @@ proc chewcommits {} {
reset_pending_select {}
if {[commitinview $pending_select $curview]} {
- selectline [rowofcommit $pending_select] 1
+ select_pending_line
} else {
Re: [PATCH] gitk: read and write a repository specific configuration file
Lukasz Stelmach wrote: > Enable gitk read and write repository specific configuration > file: ".git/k" if the file exists. To make gitk use the local > file simply create one, e.g. with the touch(1) command. I'm not sure I like this proposal. While it may be desirable to have *some* settings stored per repository, for most settings I want them to be remembered globally. Git-gui tries to solve this by presenting two panes in the preferences dialog, so that I can choose the scope of every setting I change. This still doesn't help for things that are remembered implicitly, like the window size. I don't have good suggestions how to solve this; just pointing out problems. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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: Bug in latest gitk - can't click lines connecting commits
Jason Holden wrote: > I was testing some patches against the latest gitk, and noticed that when I > click the mouse on the lines that connect the commits in the history graph, > I get an error popup with: > Error: can't read "cflist_top": no such variable > > Looks like this was introduced in gitk commit b967135d89e8d8461d059 > gitk: Synchronize highlighting in file view when scrolling diff A patch that fixes this was proposed over two months ago, and Paul said he had applied it: <http://permalink.gmane.org/gmane.comp.version-control.git/208162> However, looking at git://ozlabs.org/~paulus/gitk.git it's not there. Paul? -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] gitk: simplify file filtering
Paul Mackerras wrote:
> On Sun, May 12, 2013 at 03:54:14PM -0700, Junio C Hamano wrote:
>
> > Thanks; is this the last one for this cycle and is your usual branch
> > ready to be pulled?
>
> I'm still deciding whether to put in Martin Langhoff's patch with a
> modified label ("changing lines matching" rather than "with changes
> matching regex"). I'm leaning towards it but was waiting a little to
> see if anyone had comments on the wording. I'll decide by the end of
> today and send you a pull request either way.
Would you also consider Tair Sabirgaliev's v2 patch for not launching
gitk in the background on Mac? This fixes a very serious usability
problem.
<http://permalink.gmane.org/gmane.comp.version-control.git/43>
Thanks,
Stefan
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
--
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] git-gui: bring Wish process to front on Mac
On Mac OS X, any application that is started from the Terminal will open
behind all running applications; as a work-around, manually bring ourselves
to the front. (Stolen from gitk, commit 76bf6ff93e.)
We do this as the very first thing, so that any message boxes that might pop
up during the rest of the startup sequence are actually seen by the user.
Signed-off-by: Stefan Haller
---
git-gui.sh | 13 +
1 file changed, 13 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index e11..c464928 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -29,6 +29,19 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA}]
##
##
+## On Mac, bring the current Wish process window to front
+
+if {[tk windowingsystem] eq "aqua"} {
+ exec osascript -e [format {
+ tell application "System Events"
+ set frontmost of processes whose unix id is %d to true
+ end tell
+ } [pid] ]
+}
+
+
+##
+##
## Tcl/Tk sanity check
if {[catch {package require Tcl 8.4} err]
--
1.8.3.14.g33f718c
--
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] git-gui: bring Wish process to front on Mac
Pat Thoyts wrote:
> On 6 June 2013 09:17, Stefan Haller wrote:
> > +## On Mac, bring the current Wish process window to front
> > +
> > +if {[tk windowingsystem] eq "aqua"} {
> > + exec osascript -e [format {
> > + tell application "System Events"
> > + set frontmost of processes whose unix id is %d to
> > true
> > + end tell
> > + } [pid] ]
> > +}
>
> Seems fine to me. I can't test this as I have no access to this
> platform. Possibly you should run this in a catch statement so it can
> ignore any errors and I would tend to use the 'auto_execok' command to
> ensure that osascript actually exists. Something like
>
> set arg [format {tell application..}]
> catch {exec {*}[auto_execok osascript] -e $arg [pid]}
>
> but possibly this is guaranteed to exist on all macs which would make
> the above redundant. What I'm thinking is you dont want the app to
> exit just because something goes wrong in this call.
I don't think we need auto_execok here, as osascript is available on
every Mac system. We might even specify the exact path, it's always
/usr/bin/osascript. Is that preferable?
I agree that "catch" might be a good idea though. It raises two
questions though:
1) Should we make the same change in gitk then? It already has the same
code without the catch (released in 1.8.3 already, btw).
2) Should we think about some way of sharing code between gitk and
git gui, so that these kinds of changes don't have to be made twice?
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
--
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] git-gui: bring Wish process to front on Mac
On Mac OS X, any application that is started from the Terminal will open
behind all running applications; as a work-around, manually bring ourselves
to the front. (Stolen from gitk, commit 76bf6ff93e.)
We do this as the very first thing, so that any message boxes that might pop
up during the rest of the startup sequence are actually seen by the user.
Signed-off-by: Stefan Haller
---
Changes since the first patch:
- add catch
- specify full path to /usr/bin/osascript
git-gui.sh | 15 +++
1 file changed, 15 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index e11..a792924 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -29,6 +29,21 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA}]
##
##
+## On Mac, bring the current Wish process window to front
+
+if {[tk windowingsystem] eq "aqua"} {
+ catch {
+ exec /usr/bin/osascript -e [format {
+ tell application "System Events"
+ set frontmost of processes whose unix id is %d
to true
+ end tell
+ } [pid] ]
+ }
+}
+
+
+##
+##
## Tcl/Tk sanity check
if {[catch {package require Tcl 8.4} err]
--
1.8.3.14.g33f718c
--
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] git-gui: bring Wish process to front on Mac
Junio C Hamano wrote: > Stefan (as your name appears in 76bf6ff93e, I am assuming that you > were the OSX-osascript guru in that commit) could you keep an eye on > the list traffic to see if users of latest gitk have issues with > that change, please? Sure, will do. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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] gitk: workaround Tcl/Tk Cmd-TAB behavior on OSX
Tair Sabirgaliev wrote:
> On OSX Tcl/Tk application windows are created behind all
> the applications down the stack of windows. This is very
> annoying, because once a gitk window appears, it's the
> downmost window and switching to it is pain.
>
> The patch is trivial: if we are on OSX, emulate Cmd-Shift-TAB
> key event, so that the gitk application window is brought
> from bottom to top.
>
> Signed-off-by: Tair Sabirgaliev
> ---
>
> +# On OSX workaround the Tcl/Tk windows going down the stack of Cmd-TAB
> +if {[tk windowingsystem] eq "aqua"} {
> +exec osascript -e {
> +tell application "System Events"
> +key down command
> +key down shift
> +keystroke tab
> +key up shift
> +key up command
> +end tell+}
> +}
> +
First of all, I absolutely want this behaviour. Bringing windows up in
the background is one of the biggest usability problems of git on Mac.
However, I don't think that synthesizing the keystrokes for
Command-Shift-Tab is a good solution. It would be better to explicitly
bring our process to the front. One way to achieve this would be:
if {[tk windowingsystem] eq "aqua"} {
exec osascript -e [format {
tell application "System Events"
set frontmost of processes whose unix id is %d to true
end tell
} [pid] ]
}
(Not sure about the formatting, I don't speak Tcl...)
Also, we need the same thing in git gui as well.
Best,
Stefan
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
--
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] gitk: Synchronize highlighting in file view when scrolling diff
Whenever the diff pane scrolls, highlight the corresponding file in the
file list on the right. For a large commit with many files and long
per-file diffs, this makes it easier to keep track of what you're looking
at.
This allows simplifying the prevfile and nextfile functions, because
all they have to do is scroll the diff pane.
Signed-off-by: Stefan Haller
---
gitk | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/gitk b/gitk
index d93bd99..9e3ec71 100755
--- a/gitk
+++ b/gitk
@@ -7947,10 +7947,9 @@ proc changediffdisp {} {
$ctext tag conf dresult -elide [lindex $diffelide 1]
}
-proc highlightfile {loc cline} {
-global ctext cflist cflist_top
+proc highlightfile {cline} {
+global cflist cflist_top
-$ctext yview $loc
$cflist tag remove highlight $cflist_top.0 "$cflist_top.0 lineend"
$cflist tag add highlight $cline.0 "$cline.0 lineend"
$cflist see $cline.0
@@ -7962,17 +7961,15 @@ proc prevfile {} {
if {$cmitmode eq "tree"} return
set prev 0.0
-set prevline 1
set here [$ctext index @0,0]
foreach loc $difffilestart {
if {[$ctext compare $loc >= $here]} {
- highlightfile $prev $prevline
+ $ctext yview $prev
return
}
set prev $loc
- incr prevline
}
-highlightfile $prev $prevline
+$ctext yview $prev
}
proc nextfile {} {
@@ -7980,11 +7977,9 @@ proc nextfile {} {
if {$cmitmode eq "tree"} return
set here [$ctext index @0,0]
-set line 1
foreach loc $difffilestart {
- incr line
if {[$ctext compare $loc > $here]} {
- highlightfile $loc $line
+ $ctext yview $loc
return
}
}
@@ -8138,7 +8133,17 @@ proc searchmarkvisible {doall} {
}
proc scrolltext {f0 f1} {
-global searchstring
+global searchstring cmitmode
+global ctext cflist cflist_top difffilestart
+
+if {$cmitmode ne "tree" && [info exists difffilestart]} {
+ set top [lindex [split [$ctext index @0,0] .] 0]
+ if {$top < [lindex $difffilestart 0]} {
+ highlightfile 0
+ } else {
+ highlightfile [expr {[bsearch $difffilestart $top] + 2}]
+ }
+}
.bleft.bottom.sb set $f0 $f1
if {$searchstring ne {}} {
--
1.7.12.376.g8258bbd
--
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] gitk: Synchronize highlighting in file view when scrolling diff
Whenever the diff pane scrolls, highlight the corresponding file in the
file list on the right. For a large commit with many files and long
per-file diffs, this makes it easier to keep track of what you're looking
at.
This allows simplifying the prevfile and nextfile functions, because
all they have to do is scroll the diff pane.
Signed-off-by: Stefan Haller
---
The only change from v1 is the addition of the "$difffilestart eq {}"
condition, this should fix the flickering problem reported by Peter.
I didn't do anything about the search problem yet, will look into this
next. (Personally though, I think this is acceptable the way it is.)
gitk | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/gitk b/gitk
index d93bd99..8095806 100755
--- a/gitk
+++ b/gitk
@@ -7947,10 +7947,9 @@ proc changediffdisp {} {
$ctext tag conf dresult -elide [lindex $diffelide 1]
}
-proc highlightfile {loc cline} {
-global ctext cflist cflist_top
+proc highlightfile {cline} {
+global cflist cflist_top
-$ctext yview $loc
$cflist tag remove highlight $cflist_top.0 "$cflist_top.0 lineend"
$cflist tag add highlight $cline.0 "$cline.0 lineend"
$cflist see $cline.0
@@ -7962,17 +7961,15 @@ proc prevfile {} {
if {$cmitmode eq "tree"} return
set prev 0.0
-set prevline 1
set here [$ctext index @0,0]
foreach loc $difffilestart {
if {[$ctext compare $loc >= $here]} {
- highlightfile $prev $prevline
+ $ctext yview $prev
return
}
set prev $loc
- incr prevline
}
-highlightfile $prev $prevline
+$ctext yview $prev
}
proc nextfile {} {
@@ -7980,11 +7977,9 @@ proc nextfile {} {
if {$cmitmode eq "tree"} return
set here [$ctext index @0,0]
-set line 1
foreach loc $difffilestart {
- incr line
if {[$ctext compare $loc > $here]} {
- highlightfile $loc $line
+ $ctext yview $loc
return
}
}
@@ -8138,7 +8133,17 @@ proc searchmarkvisible {doall} {
}
proc scrolltext {f0 f1} {
-global searchstring
+global searchstring cmitmode
+global ctext cflist cflist_top difffilestart
+
+if {$cmitmode ne "tree" && [info exists difffilestart]} {
+ set top [lindex [split [$ctext index @0,0] .] 0]
+ if {$difffilestart eq {} || $top < [lindex $difffilestart 0]} {
+ highlightfile 0
+ } else {
+ highlightfile [expr {[bsearch $difffilestart $top] + 2}]
+ }
+}
.bleft.bottom.sb set $f0 $f1
if {$searchstring ne {}} {
--
1.7.12.517.g4c1112f
--
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] gitk: Synchronize highlighting in file view when scrolling diff
Whenever the diff pane scrolls, highlight the corresponding file in the
file list on the right. For a large commit with many files and long
per-file diffs, this makes it easier to keep track of what you're looking
at.
This allows simplifying the prevfile and nextfile functions, because
all they have to do is scroll the diff pane.
In some situations we want to suppress this mechanism, for example when
clicking on a file in the file list to select it, or when searching in the
diff, in which case we want to highlight based on the current search hit
and not the top line visible. In these cases it's not sufficiant to set
a "suppress" flag before scrolling and reset it afterwards, because the
scrolltext notification is sent deferred from a timer or some such; so we
need to remember the scroll position for which we want to suppress the
auto-highlighting until the next call to scrolltext; a bit ugly, but does
the job.
Signed-off-by: Stefan Haller
---
Here's one way how to address your concern. When pressing the search button
it will highlight the file that contains the current search hit; if you then
scroll from there though, the normal mechanism kicks in again and might
highlight the previous file. The same happens now if you select the last file
in the list, but it's diff is smaller than a screenful. In the previous
patch versions it would select a different file than you clicked on, which
is probably also confusing.
Is this what you had in mind?
Stefan.
gitk | 54 +++---
1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/gitk b/gitk
index d93bd99..16832a9 100755
--- a/gitk
+++ b/gitk
@@ -3309,6 +3309,7 @@ proc sel_flist {w x y} {
} else {
catch {$ctext yview [lindex $difffilestart [expr {$l - 2}]]}
}
+suppress_highlighting_file_for_current_scrollpos
}
proc pop_flist_menu {w X Y x y} {
@@ -7947,32 +7948,42 @@ proc changediffdisp {} {
$ctext tag conf dresult -elide [lindex $diffelide 1]
}
-proc highlightfile {loc cline} {
-global ctext cflist cflist_top
+proc highlightfile {cline} {
+global cflist cflist_top
-$ctext yview $loc
$cflist tag remove highlight $cflist_top.0 "$cflist_top.0 lineend"
$cflist tag add highlight $cline.0 "$cline.0 lineend"
$cflist see $cline.0
set cflist_top $cline
}
+proc highlightfile_for_scrollpos {topidx} {
+global difffilestart
+
+if {![info exists difffilestart]} return
+
+set top [lindex [split $topidx .] 0]
+if {$difffilestart eq {} || $top < [lindex $difffilestart 0]} {
+ highlightfile 0
+} else {
+ highlightfile [expr {[bsearch $difffilestart $top] + 2}]
+}
+}
+
proc prevfile {} {
global difffilestart ctext cmitmode
if {$cmitmode eq "tree"} return
set prev 0.0
-set prevline 1
set here [$ctext index @0,0]
foreach loc $difffilestart {
if {[$ctext compare $loc >= $here]} {
- highlightfile $prev $prevline
+ $ctext yview $prev
return
}
set prev $loc
- incr prevline
}
-highlightfile $prev $prevline
+$ctext yview $prev
}
proc nextfile {} {
@@ -7980,11 +7991,9 @@ proc nextfile {} {
if {$cmitmode eq "tree"} return
set here [$ctext index @0,0]
-set line 1
foreach loc $difffilestart {
- incr line
if {[$ctext compare $loc > $here]} {
- highlightfile $loc $line
+ $ctext yview $loc
return
}
}
@@ -8046,6 +8055,8 @@ proc incrsearch {name ix op} {
set here [$ctext search $searchdirn -- $searchstring anchor]
if {$here ne {}} {
$ctext see $here
+ suppress_highlighting_file_for_current_scrollpos
+ highlightfile_for_scrollpos $here
}
searchmarkvisible 1
}
@@ -8071,6 +8082,8 @@ proc dosearch {} {
return
}
$ctext see $match
+ suppress_highlighting_file_for_current_scrollpos
+ highlightfile_for_scrollpos $match
set mend "$match + $mlen c"
$ctext tag add sel $match $mend
$ctext mark unset anchor
@@ -8097,6 +8110,8 @@ proc dosearchback {} {
return
}
$ctext see $match
+ suppress_highlighting_file_for_current_scrollpos
+ highlightfile_for_scrollpos $match
set mend "$match + $ml c"
$ctext tag add sel $match $mend
$ctext mark unset anchor
@@ -8137,8 +8152,25 @@ proc searchmarkvisible {doall} {
}
}
+proc suppress_highlighting_file_for_current_scrollpos {} {
+global ctext suppress_highlighting_file_for_this_scrollpos
+
+set suppress_highlighting_file_for_this_scrollpos [$ctext index @0,0]
+}
+
proc scrolltext {f0 f1} {
-global searchstring
+global searchstring cmitmode ctext
+global suppress_highlighting_file_for_this_scrollpos
[PATCH 1/2] gitk: Highlight current search hit in orange
When searching for text in the diff, and there are multiple occurrences
of the search string, the current one is highlighted in orange, and the
other ones in yellow. This makes it much easier to understand what happens
when you then click the Search button or hit Ctrl-S repeatedly.
Signed-off-by: Stefan Haller
---
gitk | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index 16832a9..e2c0f1c 100755
--- a/gitk
+++ b/gitk
@@ -2361,6 +2361,7 @@ proc makewindow {} {
$ctext tag conf mresult -font textfontbold
$ctext tag conf msep -font textfontbold
$ctext tag conf found -back yellow
+$ctext tag conf currentsearchhit -back orange
.pwbottom add .bleft
if {!$use_ttk} {
@@ -2523,6 +2524,7 @@ proc makewindow {} {
bind $cflist $ctxbut {pop_flist_menu %W %X %Y %x %y}
bind $ctext $ctxbut {pop_diff_menu %W %X %Y %x %y}
bind $ctext {focus %W}
+bind $ctext <> rehighlight_search_results
set maincursor [. cget -cursor]
set textcursor [$ctext cget -cursor]
@@ -8039,7 +8041,6 @@ proc settabs {{firstab {}}} {
proc incrsearch {name ix op} {
global ctext searchstring searchdirn
-$ctext tag remove found 1.0 end
if {[catch {$ctext index anchor}]} {
# no anchor set, use start of selection, or of visible area
set sel [$ctext tag ranges sel]
@@ -8058,8 +8059,8 @@ proc incrsearch {name ix op} {
suppress_highlighting_file_for_current_scrollpos
highlightfile_for_scrollpos $here
}
- searchmarkvisible 1
}
+rehighlight_search_results
}
proc dosearch {} {
@@ -8087,6 +8088,7 @@ proc dosearch {} {
set mend "$match + $mlen c"
$ctext tag add sel $match $mend
$ctext mark unset anchor
+ rehighlight_search_results
}
}
@@ -8115,18 +8117,36 @@ proc dosearchback {} {
set mend "$match + $ml c"
$ctext tag add sel $match $mend
$ctext mark unset anchor
+ rehighlight_search_results
+}
+}
+
+proc rehighlight_search_results {} {
+global ctext searchstring
+
+$ctext tag remove found 1.0 end
+$ctext tag remove currentsearchhit 1.0 end
+
+if {$searchstring ne {}} {
+ searchmarkvisible 1
}
}
proc searchmark {first last} {
global ctext searchstring
+set sel [$ctext tag ranges sel]
+
set mend $first.0
while {1} {
set match [$ctext search -count mlen -- $searchstring $mend $last.end]
if {$match eq {}} break
set mend "$match + $mlen c"
- $ctext tag add found $match $mend
+ if {$sel ne {} && [$ctext compare $match == [lindex $sel 0]]} {
+ $ctext tag add currentsearchhit $match $mend
+ } else {
+ $ctext tag add found $match $mend
+ }
}
}
--
1.7.12.1.399.gae20e0d
--
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] gitk: Highlight first search result immediately on incremental search
When typing in the "Search" field, select the current search result (so
that it gets highlighted in orange). This makes it easier to understand
what will happen if you then type Ctrl-S.
Signed-off-by: Stefan Haller
---
gitk | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gitk b/gitk
index e2c0f1c..39c40de 100755
--- a/gitk
+++ b/gitk
@@ -8053,9 +8053,12 @@ proc incrsearch {name ix op} {
}
}
if {$searchstring ne {}} {
- set here [$ctext search $searchdirn -- $searchstring anchor]
+ set here [$ctext search -count mlen $searchdirn -- $searchstring anchor]
if {$here ne {}} {
$ctext see $here
+ set mend "$here + $mlen c"
+ $ctext tag remove sel 1.0 end
+ $ctext tag add sel $here $mend
suppress_highlighting_file_for_current_scrollpos
highlightfile_for_scrollpos $here
}
--
1.7.12.1.399.gae20e0d
--
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/2] gitk: Better highlighting of search results
Here's something that has been bugging me for a long time: when using the incremental search feature, it's hard to tell what happens when clicking the Search button (or type Ctrl-S) repeatedly. It does have the concept of a "current" search hit, and Ctrl-S advances to the next one; however, you can't see it because all search hits are highlighted in the same way (yellow). So when there are multiple hits visible on the current page, it will at some point scroll down to reveal more hits, but it's impossible to predict when this will happen. To improve this, we highlight the current search in orange and the other ones in yellow (like Chrome does it when you search on a Web page). Needs to go on top of the recent "Synchronize highlighting in file view when scrolling diff" patch, v3. [PATCH 1/2] gitk: Highlight current search hit in orange [PATCH 2/2] gitk: Highlight first search result immediately on incremental search -- 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] gitk: Work around empty back and forward images when buttons are disabled
On Mac, the back and forward buttons show an empty rectange instead of
a grayed-out arrow when they are disabled. The reason is a Tk bug on Mac
that causes disabled images not to draw correctly (not to draw at all,
that is); see
<https://groups.google.com/forum/?fromgroups=#!topic/comp.lang.tcl/V-nW1JBq0eU>.
To work around this, we explicitly provide gray images for the disabled
state; I think this looks better than the default stipple effect that you
get on Windows as well, but that may be a matter of taste.
Signed-off-by: Stefan Haller
---
gitk | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/gitk b/gitk
index d93bd99..e7723db 100755
--- a/gitk
+++ b/gitk
@@ -2161,7 +2161,7 @@ proc makewindow {} {
trace add variable sha1string write sha1change
pack $sha1entry -side left -pady 2
-image create bitmap bm-left -data {
+set bm_left_data {
#define left_width 16
#define left_height 16
static unsigned char left_bits[] = {
@@ -2169,7 +2169,7 @@ proc makewindow {} {
0x0e, 0x00, 0xff, 0x7f, 0xff, 0x7f, 0xff, 0x7f, 0x0e, 0x00, 0x1c, 0x00,
0x38, 0x00, 0x70, 0x00, 0xe0, 0x00, 0xc0, 0x01};
}
-image create bitmap bm-right -data {
+set bm_right_data {
#define right_width 16
#define right_height 16
static unsigned char right_bits[] = {
@@ -2177,11 +2177,16 @@ proc makewindow {} {
0x00, 0x38, 0xff, 0x7f, 0xff, 0x7f, 0xff, 0x7f, 0x00, 0x38, 0x00, 0x1c,
0x00, 0x0e, 0x00, 0x07, 0x80, 0x03, 0xc0, 0x01};
}
-${NS}::button .tf.bar.leftbut -image bm-left -command goback \
- -state disabled -width 26
+image create bitmap bm-left -data $bm_left_data
+image create bitmap bm-left-gray -data $bm_left_data -foreground "#999"
+image create bitmap bm-right -data $bm_right_data
+image create bitmap bm-right-gray -data $bm_right_data -foreground "#999"
+
+${NS}::button .tf.bar.leftbut -image [list bm-left disabled bm-left-gray] \
+ -command goback -state disabled -width 26
pack .tf.bar.leftbut -side left -fill y
-${NS}::button .tf.bar.rightbut -image bm-right -command goforw \
- -state disabled -width 26
+${NS}::button .tf.bar.rightbut -image [list bm-right disabled
bm-right-gray] \
+ -command goforw -state disabled -width 26
pack .tf.bar.rightbut -side left -fill y
${NS}::label .tf.bar.rowlabel -text [mc "Row"]
--
1.7.12.1.399.gae20e0d
--
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] gitk: Synchronize highlighting in file view when scrolling diff
Paul Mackerras wrote: > On Wed, Sep 19, 2012 at 08:17:27PM +0200, Stefan Haller wrote: > > Here's one way how to address your concern. When pressing the search button > > it will highlight the file that contains the current search hit; if you then > > scroll from there though, the normal mechanism kicks in again and might > > highlight the previous file. The same happens now if you select the last > > file > > in the list, but it's diff is smaller than a screenful. In the previous > > patch versions it would select a different file than you clicked on, which > > is probably also confusing. > > > > Is this what you had in mind? > > Yes, it is, and I applied your patch. I wonder though if it might > work better to highlight all the files that are visible? Interesting idea. I tried it, but I don't like it much, it just looks and feels so odd. I can send a patch if you're interested in trying it yourself. But personally, I really only need the synchronization feature in the case where a file's diff is longer than fits on a screen; as long as a file header is visible on the left side, it's prominent enough that I don't need more guidance. -Stefan -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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: [BUG] gitk: clicking on a connecting line produces can't read "cflist_top"
Sorry, I didn't realize that there is a display mode where the list of files is empty, not even showing a "Comments" entry. Here's a patch that fixes it, plus another patch that is only related in so far as the bug that it fixes was introduced by the same commit. [PATCH 1/2] gitk: Fix error message when clicking on a connecting line [PATCH 2/2] gitk: When searching, only highlight files when in Patch mode -- 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] gitk: Fix error message when clicking on a connecting line
When clicking on the line that connects two commit nodes, gitk
would bring up an error dialog saying "can't read "cflist_top":
no such variable".
This fixes a regression that was introduced with b967135 ("gitk:
Synchronize highlighting in file view when scrolling diff").
Signed-off-by: Stefan Haller
---
gitk | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gitk b/gitk
index 379582a..8935284 100755
--- a/gitk
+++ b/gitk
@@ -7958,6 +7958,8 @@ proc changediffdisp {} {
proc highlightfile {cline} {
global cflist cflist_top
+if {![info exists cflist_top]} return
+
$cflist tag remove highlight $cflist_top.0 "$cflist_top.0 lineend"
$cflist tag add highlight $cline.0 "$cline.0 lineend"
$cflist see $cline.0
--
1.8.0.rc0.36.gef0f079
--
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] gitk: When searching, only highlight files when in Patch mode
This fixes another regression that was introduced in b967135 ("gitk:
Synchronize highlighting in file view when scrolling diff"): when
searching for a string in tree mode, jumping to the next search hit
would highlight the "Comments" entry in the file list.
Signed-off-by: Stefan Haller
---
gitk | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/gitk b/gitk
index 8935284..0ee5585 100755
--- a/gitk
+++ b/gitk
@@ -7967,8 +7967,9 @@ proc highlightfile {cline} {
}
proc highlightfile_for_scrollpos {topidx} {
-global difffilestart
+global cmitmode difffilestart
+if {$cmitmode eq "tree"} return
if {![info exists difffilestart]} return
set top [lindex [split $topidx .] 0]
@@ -8192,12 +8193,10 @@ proc scrolltext {f0 f1} {
global searchstring cmitmode ctext
global suppress_highlighting_file_for_this_scrollpos
-if {$cmitmode ne "tree"} {
- set topidx [$ctext index @0,0]
- if {![info exists suppress_highlighting_file_for_this_scrollpos]
- || $topidx ne $suppress_highlighting_file_for_this_scrollpos} {
- highlightfile_for_scrollpos $topidx
- }
+set topidx [$ctext index @0,0]
+if {![info exists suppress_highlighting_file_for_this_scrollpos]
+ || $topidx ne $suppress_highlighting_file_for_this_scrollpos} {
+ highlightfile_for_scrollpos $topidx
}
catch {unset suppress_highlighting_file_for_this_scrollpos}
--
1.8.0.rc0.36.gef0f079
--
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] gitk: Do not select file list entries during diff loading
Peter Oberndorfer wrote:
> Please review/test the patch carefully before applying,
> since i do not often work with tcl/tk :-)
The patch makes perfect sense to me. (I'm not a great tcl coder either
though, and not very familiar with the gitk code; so another review
would be helpful.)
Just one minor suggestion:
> proc scrolltext {f0 f1} {
> -global searchstring cmitmode ctext
> +global searchstring cmitmode ctext ctext_last_scroll_pos
> global suppress_highlighting_file_for_this_scrollpos
>
> +.bleft.bottom.sb set $f0 $f1
> +if {$searchstring ne {}} {
> +searchmarkvisible 0
> +}
> +
> set topidx [$ctext index @0,0]
> +if {$topidx eq $ctext_last_scroll_pos} return
> +set ctext_last_scroll_pos $topidx
> +
> if {![info exists suppress_highlighting_file_for_this_scrollpos]
> || $topidx ne $suppress_highlighting_file_for_this_scrollpos} {
> highlightfile_for_scrollpos $topidx
> }
>
> catch {unset suppress_highlighting_file_for_this_scrollpos}
> -
> -.bleft.bottom.sb set $f0 $f1
> -if {$searchstring ne {}} {
> -searchmarkvisible 0
> -}
> }
I don't like early returns, they can easily become a source of bugs when
someone adds more code to the end of a function without realizing that
there's an early return in the middle. I'd much rather say something
like
if {$topidx ne $ctext_last_scroll_pos} {
if {![info exists suppress_highlighting_file_for_this_scrollpos]
|| $topidx ne $suppress_highlighting_file_for_this_scrollpos} {
highlightfile_for_scrollpos $topidx
}
set ctext_last_scroll_pos $topidx
}
-Stefan
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
--
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: Feature request: fetch --prune by default
Dan Johnson wrote: > In the meantime, would it make sense to introduce a configuration > variable to request this behavior? > > fetch.prune = always > > Of course, this might be just a waste of time to introduce a feature > no one would use, in which case we obviously should not introduce such > options. I would use it. -- Stefan Haller Berlin, Germany http://www.haller-berlin.de/ -- 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
