Re: [PATCH 2/3] gitk: write only changed configuration variables

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 2/3] gitk: write only changed configuration variables

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:29

Max Kirillov [off-list ref] 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?
quoted hunk
Signed-off-by: Max Kirillov <redacted>
---
 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.

Re: [PATCH 2/3] gitk: write only changed configuration variables

From: Max Kirillov <hidden>
Date: 2016-06-15 23:02:29

On Thu, Sep 11, 2014 at 10:19:56AM -0700, Junio C Hamano wrote:
Max Kirillov [off-list ref] writes:
quoted
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."
Something like:

"""
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.

After the change, a parameter is stored in config only it is
has been modified in the exiting instance. Otherwise, the
value which currently is in file is preserved. This allows
editing the configuration when several instances are
running, and don't get rollback of the modification if some
other instance where the cinfiguration was not edited is
closed last.
"""

Does it looks appropriate?

(Actually, the main motivation was the 3/3 part, for views,
scalar parameters merging was just low hanging fruit by the
way)
quoted
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?
Sure. Thanks
quoted
+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.
This is going to complicate UI, something like "closing
confirmation dialog". Not nice. And, I am actually not sure
it is really needed, because "the other party" is me again,
in another gitk window, and most probably I would want the
same change.

Though storing the old value and comparing to it makes sanse
to do anyway, because trace may produce bogus events, so it
would be better to doublecheck has the value actually been
changed.

-- 
Max
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help