Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Document that WinResize executes before window_* expansions are updated. #4994

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Document that WinResize executes before window_* expansions are updated. #4994

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions doc/pages/hooks.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,12 @@ name. Hooks with no description will always use an empty string.
a window was destroyed. This hook is executed in a draft context, so any
changes to selections or input state will be discarded.

*WinResize* `<line>.<column>`::
a window was resized. This hook is executed in a draft context, so any
changes to selections or input state will be discarded.
*WinResize* `<new_lines>.<new_columns>`::
a window was resized. While this hook executes, the `window_*` value
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem correct, from the code we should have the new value in those.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevertheless, that seems to be how it works. Using the current version (5c793a6), I launched Kakoune with:

kak -n -e "hook global WinResize .* %{ echo -debug window_range: %val{window_range} hook_param: %val{hook_param} }; buffer *debug*"

In the *debug* buffer, I saw:

window_range: 0 0 27 0 hook_param: 27.106

...which matches the expected dimensions of my terminal. If I hit the "toggle fullscreen" shortcut, I get:

window_range: 0 0 27 0 hook_param: 58.213

...where the hook_param is the new dimensions, and window_range is the old dimensions. If I toggle fullscreen again:

window_range: 0 0 58 0 hook_param: 27.106

...once again hook_param is the new dimensions, and window_range is the old.

I figured this out while investigating #4975

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is specific to window_range though, if you do:

kak -n -e "hook global WinResize .* %{ echo -debug window_range: %val{window_range} window_height: %val{window_height} window_width: %val{window_width} hook_param: %val{hook_param} }; buffer *debug*"

You will see window_width and window_height match the hook parameter.

window_range is different because it gives you what range of the buffer is currently displayed. It used to compute this on demand and mostly matched what the next redraw would look like, it now accesses the range from last redraw and hence might not match the next redraw.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see!

How should I document this? Should this interaction between WinResize and window_range be documented here, or in the window_range docs?

Alternatively, should WinResize be modified to run after the first redraw with the new dimensions, so that all the values are up-to-date?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm it seems to me that window_range should match the next redraw. That would be more correct though I don't know if there is a practical difference besides the initialization. Maybe that's awkward to implement.

I guess as a stop-gap solution we could throw an error if window_range is not initialized.
Then users could do try %{ set-register r %val{window_range} }.
It's still ugly because it's such a special case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess as a stop-gap solution we could throw an error if window_range is not initialized.

Feels wrong but I wonder if it works for practical uses

diff --git a/src/exception.hh b/src/exception.hh
index 28f410291..b5c91184a 100644
--- a/src/exception.hh
+++ b/src/exception.hh
@@ -34,6 +34,11 @@ struct cancel : runtime_error
     cancel() : runtime_error("cancellation requested") {}
 };
 
+struct undefined_value : runtime_error
+{
+    using runtime_error::runtime_error;
+};
+
 struct logic_error : exception
 {
 };
diff --git a/src/main.cc b/src/main.cc
index ca18760d7..4973d4fae 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -388,8 +388,10 @@ static const EnvVarDesc builtin_env_vars[] = { {
         [](StringView name, const Context& context) -> Vector<String>
         {
             const auto& setup = context.window().last_display_setup();
-            return {format("{} {} {} {}", setup.first_line, setup.first_column,
-                                          setup.line_count, 0)};
+            if (not setup)
+                throw undefined_value{"window was not rendered yet, no window_range in context"};
+            return {format("{} {} {} {}", setup->first_line, setup->first_column,
+                                          setup->line_count, 0)};
         }
     }, {
         "history", false,
diff --git a/src/shell_manager.cc b/src/shell_manager.cc
index 62a60dc29..8ff874309 100644
--- a/src/shell_manager.cc
+++ b/src/shell_manager.cc
@@ -162,6 +162,8 @@ Vector<String> generate_env(StringView cmdline, const Context& context, GetValue
             StringView quoted{match[1].first, match[1].second};
             Quoting quoting = match[1].matched ? Quoting::Shell : Quoting::Raw;
             env.push_back(format("kak_{}{}={}", quoted, name, get_value(name, quoting)));
+        } catch (undefined_value&) {
+            throw;
         } catch (runtime_error&) {}
     }
 
diff --git a/src/window.hh b/src/window.hh
index 3db3dcc2e..b37361fc0 100644
--- a/src/window.hh
+++ b/src/window.hh
@@ -49,7 +49,7 @@ public:
     void clear_display_buffer();
     void run_resize_hook_ifn();
 
-    const DisplaySetup& last_display_setup() const { return m_last_display_setup; }
+    const Optional<DisplaySetup>& last_display_setup() const { return m_last_display_setup; }
 
 private:
     Window(const Window&) = delete;
@@ -70,7 +70,7 @@ private:
 
     Highlighters m_highlighters;
     bool m_resize_hook_pending = false;
-    DisplaySetup m_last_display_setup;
+    Optional<DisplaySetup> m_last_display_setup;
 
     struct Setup
     {

expansions contain the old size
(see <<expansions#value-expansions,`:doc expansions value-expansions`>>),
This hook is executed in a draft context, so any changes to selections or
input state will be discarded.

*WinDisplay* `buffer name`::
a client switched to displaying the given buffer.
Expand Down