From 16908bf0914e012c4f68de5d875e169f61f2cebd Mon Sep 17 00:00:00 2001
From: Maxime Coste <mawww@kakoune.org>
Date: Wed, 28 Nov 2018 21:45:40 +1100
Subject: [PATCH] Fix hooks triggering on unset-option even if parent has same
 value

---
 src/option_manager.cc                                     | 8 ++++++--
 src/option_manager.hh                                     | 7 +++++++
 test/compose/no-hook-on-unset-option-with-same-parent/cmd | 1 +
 test/compose/no-hook-on-unset-option-with-same-parent/in  | 1 +
 test/compose/no-hook-on-unset-option-with-same-parent/out | 1 +
 test/compose/no-hook-on-unset-option-with-same-parent/rc  | 5 +++++
 6 files changed, 21 insertions(+), 2 deletions(-)
 create mode 100644 test/compose/no-hook-on-unset-option-with-same-parent/cmd
 create mode 100644 test/compose/no-hook-on-unset-option-with-same-parent/in
 create mode 100644 test/compose/no-hook-on-unset-option-with-same-parent/out
 create mode 100644 test/compose/no-hook-on-unset-option-with-same-parent/rc

diff --git a/src/option_manager.cc b/src/option_manager.cc
index de16cf24..ea7a1d91 100644
--- a/src/option_manager.cc
+++ b/src/option_manager.cc
@@ -80,10 +80,14 @@ const Option& OptionManager::operator[](StringView name) const
 void OptionManager::unset_option(StringView name)
 {
     kak_assert(m_parent); // cannot unset option on global manager
-    if (m_options.contains(name))
+    auto it = m_options.find(name);
+    if (it != m_options.end())
     {
+        auto& parent_option = (*m_parent)[name];
+        const bool changed = not parent_option.has_same_value(*it->value);
         m_options.erase(name);
-        on_option_changed((*m_parent)[name]);
+        if (changed)
+            on_option_changed(parent_option);
     }
 }
 
diff --git a/src/option_manager.hh b/src/option_manager.hh
index 6c9db2ff..372c04d2 100644
--- a/src/option_manager.hh
+++ b/src/option_manager.hh
@@ -59,6 +59,8 @@ public:
     virtual void add_from_strings(ConstArrayView<String> strs) = 0;
     virtual void update(const Context& context) = 0;
 
+    virtual bool has_same_value(const Option& other) const = 0;
+
     virtual Option* clone(OptionManager& manager) const = 0;
     OptionManager& manager() const { return m_manager; }
 
@@ -167,6 +169,11 @@ public:
     {
         option_update(m_value, context);
     }
+
+    bool has_same_value(const Option& other) const override
+    {
+        return other.is_of_type<T>() and other.get<T>() == m_value;
+    }
 private:
     virtual void validate(const T& value) const {}
     T m_value;
diff --git a/test/compose/no-hook-on-unset-option-with-same-parent/cmd b/test/compose/no-hook-on-unset-option-with-same-parent/cmd
new file mode 100644
index 00000000..8b137891
--- /dev/null
+++ b/test/compose/no-hook-on-unset-option-with-same-parent/cmd
@@ -0,0 +1 @@
+
diff --git a/test/compose/no-hook-on-unset-option-with-same-parent/in b/test/compose/no-hook-on-unset-option-with-same-parent/in
new file mode 100644
index 00000000..8b137891
--- /dev/null
+++ b/test/compose/no-hook-on-unset-option-with-same-parent/in
@@ -0,0 +1 @@
+
diff --git a/test/compose/no-hook-on-unset-option-with-same-parent/out b/test/compose/no-hook-on-unset-option-with-same-parent/out
new file mode 100644
index 00000000..257cc564
--- /dev/null
+++ b/test/compose/no-hook-on-unset-option-with-same-parent/out
@@ -0,0 +1 @@
+foo
diff --git a/test/compose/no-hook-on-unset-option-with-same-parent/rc b/test/compose/no-hook-on-unset-option-with-same-parent/rc
new file mode 100644
index 00000000..616980d1
--- /dev/null
+++ b/test/compose/no-hook-on-unset-option-with-same-parent/rc
@@ -0,0 +1,5 @@
+decl str foo
+hook global BufSetOption foo=.* %{ exec ifoo<esc> }
+set-option buffer foo value # triggers hook
+set-option global foo value # does not trigger hook
+unset-option buffer foo # does not trigger hook