@b4n commented on this pull request.


> @@ -1022,6 +1022,42 @@ void ui_sidebar_show_hide(void)
 }
 
 
+void ui_menubar_show_hide(void)

I don't really like this function's implementation.  Usually our `show_hide()` 
calls don't behave like `toggle()`: you can call them as many times as you want 
and it'll just make sure stuff is consistent.

So I'd rather see a either caller passing the requested value (e.g. result of 
`gtk_check_menu_item_get_active()`, or `! ui_prefs.menubar_visible`) as 
parameter (like `ui_widget_show_hide()`) and the function deciding if it's OK 
and synchronizing what needs to be; or the caller having to manually toggle 
`ui_prefs.menubar_visible` and use that as teh "parameter" above.

What do you think?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/4279#pullrequestreview-2909553583
You are receiving this because you are subscribed to this thread.

Message ID: <geany/geany/pull/4279/review/[email protected]>

Reply via email to