Skip to content

SearchBar: set method → properties, rm unused - #1808

Merged
jeremypw merged 1 commit into
masterfrom
danirabbit/searchbar-simplifyproperties
Aug 25, 2026
Merged

SearchBar: set method → properties, rm unused#1808
jeremypw merged 1 commit into
masterfrom
danirabbit/searchbar-simplifyproperties

Conversation

@danirabbit

Copy link
Copy Markdown
Member
  • reveal () and is_revealed { get; } become search_mode_enabled { get; set; }
  • set_search_entry_text () and entry_text { get; } become search_text { get; set; }
  • Remove one line search_is_focused { get; } and replace_is_focused { get; } which are only used in one place anyways and on_text_buffer_changed () which just calls update_search_widgets ()
  • Remove unused transition_time_msec { get; } and highlight_none ()

@danirabbit
danirabbit requested a review from a team August 25, 2026 01:29

private bool on_key_pressed (uint keyval, uint keycode, Gdk.ModifierType state) {
if (!(search_is_focused || replace_is_focused)) {
if (!(search_entry.has_focus || replace_entry.has_focus)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can this be replaced by if (!is_focused) {?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It could but I'm not really a fan of using external API internally when it's just a one-liner like this. If the public function is removed then we'll just have this one liner function hanging on until someone else comes to clean it up again and I don't think it really makes the code more clear personally. But if you feel strongly about it, I'll change it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK as it is done for a reason I withdraw the suggestion.

@jeremypw jeremypw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Works as before afaict. Just a small code suggestion

@jeremypw
jeremypw merged commit b2f7156 into master Aug 25, 2026
6 checks passed
@jeremypw
jeremypw deleted the danirabbit/searchbar-simplifyproperties branch August 25, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants