From: Paul Mackerras <paulus@samba.org>
To: Max Kirillov <max@max630.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v4 1/2] gitk: write only changed configuration variables
Date: Mon, 2 Mar 2015 10:47:30 +1100 [thread overview]
Message-ID: <20150301234729.GB24862@iris.ozlabs.ibm.com> (raw)
In-Reply-To: <1415571602-5858-2-git-send-email-max@max630.net>
On Mon, Nov 10, 2014 at 12:20:01AM +0200, Max Kirillov wrote:
> When gitk contains some changed parameter, and there is existing
> instance of gitk where the parameter is still old, it is reverted to
> that old value when the instance exits.
>
> Instead, store a parameter in config only it is has been modified in the
> exiting instance. Otherwise, preserve the value which currently is in
> file. This allows editing the configuration when several instances are
> running, and don't get rollback of the modification if some other
> instance where the configuration was not edited is closed last.
>
> For scalar variables, use trace(3tcl) to detect their change. Since `trace` can
> send bogus events, doublecheck if the value has really been changed, but once
> it is marked as changed, do not reset it back to unchanged ever, because if
> user has restored the original value, it's the decision which should be stored
> as well as modified value.
>
> Treat view list especially: instead of rewriting the whole list, merge
> individual views. Place old and updated views at their older placed, add
> new ones to the end of list. Collect modified view explicitly, in newviewok{}
> and delview{}.
>
> Do not merge geometry values. They are almost always changing because
> user moves and resises windows, and there is no way to find which one of
> the geometries is most desired. Just overwrite them unconditionally,
> like earlier.
>
> Signed-off-by: Max Kirillov <max@max630.net>
Looks pretty nice; I just have one comment:
> + lappend views_modified_names $current_viewname($v)
This view_modified_names variable doesn't seem to be used anywhere.
If you don't mind me taking out this line, I'll do that and apply the
patch.
Regards,
Paul.
next prev parent reply other threads:[~2015-03-02 0:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-09 22:20 [PATCH v4 0/2] gitk: save only changed configuration on exit Max Kirillov
2014-11-09 22:20 ` [PATCH v4 1/2] gitk: write only changed configuration variables Max Kirillov
2015-03-01 23:47 ` Paul Mackerras [this message]
2015-03-02 20:34 ` Max Kirillov
2014-11-09 22:20 ` [PATCH v4 2/2] gitk: synchronize config write Max Kirillov
2015-03-02 0:10 ` Paul Mackerras
2015-03-02 20:43 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150301234729.GB24862@iris.ozlabs.ibm.com \
--to=paulus@samba.org \
--cc=git@vger.kernel.org \
--cc=max@max630.net \
/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.
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 NNTP newsgroup(s).