git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Max Kirillov <max@max630.net>
Cc: Paul Mackerras <paulus@samba.org>, git@vger.kernel.org
Subject: Re: [PATCH 2/3] gitk: write only changed configuration variables
Date: Thu, 11 Sep 2014 10:19:56 -0700	[thread overview]
Message-ID: <xmqqtx4edqgz.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1410412888-10187-3-git-send-email-max@max630.net> (Max Kirillov's message of "Thu, 11 Sep 2014 08:21:27 +0300")

Max Kirillov <max@max630.net> writes:

> If a variable is changed in a concurrent gitk or manually it is
> preserved unless it has changed in this instance

It would have been easier to understand why this is a desirable
change if you stated what problem you are trying to solve before
that sentence.  "If I do X, Y happens, which is bad for reason Z.
With this change, Y no longer happens as long as I do not do W."

> This change does not affect geometry and views save; geometry does not
> need it, and views need special merging, which treats each view
> separately rather that fully replace the shole list.

s/sh/wh/ I presume?

>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
>  gitk | 41 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 8 deletions(-)
>
> diff --git a/gitk b/gitk
> index 6069afe..6e22024 100755
> --- a/gitk
> +++ b/gitk
> @@ -2804,12 +2804,25 @@ proc doprogupdate {} {
>      }
>  }
>  
> +proc config_variable_change_cb {name name2 op} {
> +    global config_variable_changed
> +    if {$op eq "write"} {
> +	set config_variable_changed($name) 1
> +    }
> +}

Hmm, wouldn't it make more sense to save the original value where
you set up the variable trace, and make the savestuff procedure do a
3-way merge?  That way, when you and the other party changed a
variable to a different value, you can give a better diagnosis to
the user to know what is going on. If both of you changed to the
same value, then the end result would be the same, of course.

  reply	other threads:[~2014-09-11 17:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-11  5:21 [PATCH 0/3] gitk: save only changed configuration on exit Max Kirillov
2014-09-11  5:21 ` [PATCH 1/3] gitk refactor: remove boilerplate for configuration variables Max Kirillov
2014-09-11  5:21 ` [PATCH 2/3] gitk: write only changed " Max Kirillov
2014-09-11 17:19   ` Junio C Hamano [this message]
2014-09-11 19:17     ` Max Kirillov
2014-09-11 20:26       ` Junio C Hamano
2014-09-11  5:21 ` [PATCH 3/3] gitk: merge views with existing ones Max Kirillov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtx4edqgz.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=max@max630.net \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).