Nice.

But agree with @codebrainz that it should be optional or a plugin and from the screenshot it doesn't seem to have all the options the find dialog has.

Also why not add more options to the toolbar which already has a search field?

@js361014 any reason you feel this is better as a core feature than a plugin?

I don't disagree with the search bar vs search dialog (for most cases), I'm just curious if there are any restrictions in the plugin API that make it infeasible as an extension to Geany rather than replacing the core feature wholesale.

I support this, and looking at the relatively minor modifications, I don't see this as a plugin. Instead, I would rather see an three-way option (top, bottom or dialog). Search bar at the bottom is probably more widely adopted in other programs.

It looks like you re-use the dialog's interface widgets, which is kind of neat, as it simplifies supporting both options. However, the search bar looks a bit muddled in the screen shot.

@elextr It doesn't eat up much vertical space. I would rather remove simplified search box from the toolbar bacause it won't be needed any more - instead you could use Ctrl+F shortcut and search bar. @kugel- in case of bottom position of search bar what do you expect to happen on clicking "More" expander?

If its on top it should be part of the toolbar (where there already is a simplified search box) and so it can be put on the menu line so it doesn't eat up vertical space, or cause the text to move by appearing and disappearing in the notebook page.

If its on the bottom, maybe it should be optional to be combined with the status bar.

I disagree. The search bar comes and goes, the toolbar and statusbar are static. Sidebar and message window (on the right in my case) jumping around is distracting. I see that the search bar is part of the editor just like the header bar (when files change externally).

@js361014 if the expander simply expands a second half I would suggest it expands over the text entry so that the text entry doesn't jump.

The search bar comes and goes

Ahh, I meant for it to be permanent like those bars are

Sidebar and message window (on the right in my case) jumping around is distracting

How does this apply to having the searchbar in the status or toolbar? Even if its not permanent both status bar and tool bar extend the full width irrespective of where the sidebar or message window are positioned.

And its this full width that I am suggesting be utilised not wasted.

@b4n

- Good point. - Search bar won't support only "find all in session" part. I guess it is a job for "Find in Files" (Shift+Ctrl+F) dialog, which could be improved in future as well.

@elextr but it would be easy to add an option to the dialog to search in session instead of in specific directory

@kugel-

Why?

Besides "In Session" there are "Mark" and "In Document" options in "Find All" part. I implemented auto marking all occurrences in document. "In Document" lists occurrences in messages tab - I can implement option for it in search bar with the same behavior.

Also, find in files is a lot different. It calls an external program which has its own syntax rules, while find in session just expand "find all in document" to all open files, without external program.

It's just a matter of implementation of searching in all documents. It can be changed.

d) Next time the user types ctrl-f

@kugel- good point, d) applies when the option is turned on, e) applies when the option is turned off

codebrainz commented on this pull request.

• vbox = gtk_widget_get_ancestor(page, GTK_TYPE_VBOX);

+ GList *vbox_children = gtk_container_get_children(GTK_CONTAINER(vbox)); + GtkWidget *sbox; + if (interface_prefs.search_bar_position == SEARCH_BAR_POSITION_TOP) + { + sbox = (GtkWidget*)vbox_children->data; + } + else + { + sbox = (GtkWidget*)((GList*)g_list_last(vbox_children))->data; + } + g_list_free(vbox_children); + return sbox; +} + +void ui_emit_entry_what_to_search_in_search_bar_changed_if_visible(GeanyDocument *doc)

:astonished:

Not to bikeshed, but it might be useful to spend some of those characters explaining what `sbox` is inside the function. Perhaps it could be shortened to `ui_emit_search_bar_entry_changed`? Likewise for below, it could be something like `ui_emit_search_bar_close_button_clicked`. While I'm a big fan of descriptive function names, it doesn't necessarily have to say everything the function does.

While I'm not strictly opposed to this in core, as far as I can see, nobody has given a reason (which doesn't apply equally to any plugin) why this doesn't make more sense as a plugin. I've done it myself, it's certainly doable.

Personally, I'd rather see the 3 search dialogs unified (code-wise) and made to use Glade, and leave all other kinds of search (toolbar, search bars, etc.) to plugins.

@js361014 it's not _my_ philosophy per se, just a sort of convention.

Examples of plugins which improve, extend, or replace builtin features, which one could equally argue ought to be in core: * [Addons](https://plugins.geany.org/addons.html) has quite a few little things. * [Autoclose](https://plugins.geany.org/autoclose.html) fixes Geany's not great brace closing to make it more like other editors. * [Automark](https://plugins.geany.org/automark.html) fixes Geany's "Mark All" feature to work more like other editors. * [Commander](https://plugins.geany.org/commander.html) extends traditional menus/bindings with a "palette" thing like VSCode, Sublime, and others have builtin. * These ones try to fix/improve Geany's project management: - [Geanyprj](https://plugins.geany.org/geanyprj.html) - [ProjectOrganizer](https://plugins.geany.org/projectorganizer.html) - [Workbench](https://plugins.geany.org/workbench.html) * [Multiterm](https://plugins.geany.org/multiterm.html) tries to fix/improve the builtin terminal. * [TreeBrowser](https://plugins.geany.org/treebrowser.html) tries to fix/improve the builtin FileBrowser, which is itself a plugin (shipped with core). * [VimMode](https://plugins.geany.org/vimode.html) replaces/extends the builtin keybindings/navigation to be more like Vim.

The pattern/tradition here, is that if you don't like something in Geany core (especially if it's a matter of taste, and optional anyway, like this PR), then you make a plugin, and ideally add it to the [Geany-Plugins](https://github.com/geany/geany-plugins) project, or even try to get it added as a core plugin.

I just don't see how the feature/functionality of this PR is any different than many plugins.

IMO we try to offload too many stuff to plugins (just by suggesting "this could be in a plugin"). Requiring the user to discover and enable plugins for even the most basic enhancements degrades Geany itself, and makes the plugin manager become a mess.

Plus, those "just add one tiny enhancement" plugins are often fire-and-forget and become orphaned and unmaintained.

This doesn't service our users well. Plugins should add complex functionality that the core devs aren't up to maintain. Having tons of micro plugins is worse overall than just adding 50-100 LOCs to the core (ideally along with some unit tests).

js361014 commented on this pull request.

• vbox = gtk_widget_get_ancestor(page, GTK_TYPE_VBOX);

+ GList *vbox_children = gtk_container_get_children(GTK_CONTAINER(vbox)); + GtkWidget *sbox; + if (interface_prefs.search_bar_position == SEARCH_BAR_POSITION_TOP) + { + sbox = (GtkWidget*)vbox_children->data; + } + else + { + sbox = (GtkWidget*)((GList*)g_list_last(vbox_children))->data; + } + g_list_free(vbox_children); + return sbox; +} + +void ui_emit_entry_what_to_search_in_search_bar_changed_if_visible(GeanyDocument *doc)

@codebrainz Done.

Requiring the user to discover and enable plugins for even the most basic enhancements degrades Geany itself, and makes the plugin manager become a mess.

You're making an argument that the current UX for plugins sucks, not that this PR specifically shouldn't be a plugin.

Plus, those "just add one tiny enhancement" plugins are often fire-and-forget and become orphaned and unmaintained.

So dumping all of those unmaintained fire-and-forget codes into core is better?

Plugins should add complex functionality that the core devs aren't up to maintain. Having tons of micro plugins is worse overall than just adding 50-100 LOCs to the core (ideally along with some unit tests).

Not IMO. Aside from the friction to install/update/remove plugins, there's not really any good reason not to have nearly everything in plugins. As mentioned, this isn't a reason to bring a bunch of plugins into core functionality, it's a reason to improve the plugin experience.

Anyway, I'm not opposed to doing what the PR title says ("Replace search dialog with search bar"), I'm just opposed to adding _another_ search UI in addition to what exists, hidden behind a preference. If the search bar is truly superior to everyone, then we should remove the search dialog and replace it with the search bar, otherwise, it ought to be a plugin, IMO.

Seach bar will be a default option, so search dialog would be a "addition". I treat not to replace search dialog with search bar right away as transition step.

To play devil's advocate, I have 3 HD monitors, one of them is 4k resolution, I'd much prefer to have the search in separate dialog I can move to another monitor. Are you really going to deprecate/remove the advanced, separate window search dialog, breaking my workflow? NEVAR!!!! --some users

@elextr

To just come into a project and say "here, this is the way it should be done, throw away your existing features" is not the way to get engagement in a diverse group of contributors.

At first step I posted a screenshot waiting for opinions from contributors. I feel @kugel- and @b4n supported my idea of search bar. Moreover, I modified my original code so that user can chose from 3 options: search bar on top, search bar at bottom and an old search dialog. I feel you used a straw man argument.

like expanding the toolbar search facility instead of using space in the editor, which you havn't addressed yet

I guess it would be a long discussion on what should be in a toolbar, which of toolbar thinks should be provided as plugins etc. I felt that after @kugel- explanation

The search bar comes and goes, the toolbar and statusbar are static. Sidebar and message window (on the right in my case) jumping around is distracting. I see that the search bar is part of the editor just like the header bar (when files change externally).

there is no need to elaborate.

You need to make a case, and just saying "its the way browsers do it" isn't particularly persuasive when talking about an IDE.

I provided some arguments for search bar in https://github.com/geany/geany/pull/2220#issuecomment-512686843 and @codebrainz said

I don't disagree with the search bar vs search dialog (for most cases)

so I thought that's enough.

@elextr @codebrainz Can you list what still is not clear, what needs to be discussed/elaborated?

Maybe making cross button smaller and using tooltips would be a solution? Or making search input a little bit narrower? 

What about this? 

Am 30. Juli 2019 01:14:57 MESZ schrieb Matthew Brush notifications@github.com:

@js361014 my only issue with the actual change is that it doesn't include the Search and Replace, so now there's different UIs for normal find (dialog, search bar, toolbar) and Replace (dialog) and Find In Files (dialog).

True, in my view the bar would entirely replace the dialog (which is driving me nuts on my 3 monitor setup), i.e. the dialog ui would move into the bar, probably with some tweaks. I wouldn't except yet another search UI if it cannot fully replace an existing one.

Other than that I'm just unclear why this has to go into core instead of a plugin like would normally be done. I'd like to hear from some other core devs whether we're going to start bringing in additions to core which could/should go in plugins. @b4n @eht16 @frlan @ntrel?

Its always a case-by-case discussion, there is no general rule that every enhancement that can possibly be implemented in a plugin must be implemented in a plugin. The author would rather see it in the core so thats what he has spent his time on. I support him in this case because I hate the dialog and the "offload small but useful features to fire-and-forget plugins" methodology in general.

What's is the current status of the PR? Is it under review, waiting for review?

@elextr I know that. I just want to make sure that it's not like somebody is waiting for some my action, so it's blocking moving things forward.

Maybe I missed something but on my system it doesn't seem to work. Hitting ctrl+f does nothing, wit h either top or bottom options. With the "none" option the dialog appears.

Also, some more opinions from my side: 1) I would accept this only if it can replace the search dialog entirely, including replace functionality. 2) Since it's a new UI for existing functionality there shouldn't be much code churn 3) While I agree to enable it by default, I think existing installations shouldn't be switched over.

I'm running Arch Linux, Geany on GTK+3 3.24.10, glib 2.60.6

I suggested in #2220 (comment) to provide it in another PR. Is it OK to you?

Not really. It can be separate commits here but the change should be done wholesale.

Since it's a new UI for existing functionality there shouldn't be much code churn

Maybe we aren't on the same page. In my view, the search bar simply moves the dialog contents into a bar (e.g. GtkInfoBar or something custom) and for the rest of the code it should be pretty transparent whether the bar or dialog is shown.

Ideally all would be handled in the glade xml and search.c. notebook.c shouldn't be concerned too much (I remember there is the assumption that the ScintillaWidget of a GeanyDocument is a direct child in the hierarchy, that'd need to be lifted).

How to achieve this?

I doubt we have a generic solution, so you have to come up with something :-) perhaps some meta-preference that isn't shown in the UI just to indicate the new-installation status.

Do we really need to do this?

Yes.

Search and replace dialog and search dialog are now different dialogs. I guess search bar and replace bar and search bar could be separated as well (obviously, they can use some same functions).

Err, yes, you are right. I didn't have enough coffee it seems. So the bars should be separate as well (thus can be a separate PR). Combining the dialogs (and consequently the bars) is a different topic then.

It's strange that we add something, make it default, and then we hide from in existing installations.

The thing is, a lot of users probably won't like the new search bar, so it doesn't seem very polite to thrust it on them. This is another reason it would make more sense as a plugin.

Discussion on it looks like a never-ending story.

Yeah, ideally it happens before the pull request on the developers mailing list.

Of course, it's the question who and how to ask.

I guess the user's mailing list is the best/only place, at least if anyone is strongly for or against it they might chime-in.

It's strange that we add something, make it default, and then we hide from in existing installations.

Since it's a major UI change (search is a heavily function) I would rather not force this surprise on "senior users". Interested users can change the setting and new users get the bar anyway.

I don't feel that "asking users" will get us anywhere. We don't have a forum where we can reach a significant portion, geany-users is the best bet but reflects only a small subset. Even then, most probably can't be bothered to specifically test this PR and report back.

@kugel-

Since it's a major UI change (search is a heavily function) I would rather not force this surprise on "senior users".

I guess it would be written in change log, so it won't be surprise. Changes like this (sometimes even more major) happens in many applications and I have never ever seen something like hiding new feature in existing installations. It is normal that after an update something changes in application and sometimes an action is needed to adjust a new feature.

Please explain to me why there is a need for the large diff in notebook.c since this should just shuffle widgets around.

There is no one global search bar, so it's not just shuffling widgets around. In a rough approximation: - 30% relates to a need to pass search bar elements to each other - 10% relates to performing a searching - 10% relates to attaching signals to elements - 30% relates to updating the search bar state - 20% relates to building the search bar

kugel- requested changes on this pull request.

I finally found time to give this a try again.

What I like is that the search bar doesn't use much space, and the "mark-as-you-type" feature that gives direct feedback to what you type.

I dislike that, while space-efficient, it looks too crammed. Some spacing is needed. Also it doesn't scale down well to small window sizes.

But it's definitely something I'd like to use over the dialog for everyday usage.

But the code is unacceptable in its current form I'm afraid. - mark-as-you-type is really nice but it bloats the diff currently. Chances for merging the general feature are better if it has parity with the dialog, and new features are added separately (and also to the dialog) - The bar UI is completely C coded, this should be in glade as much as possible - hacking this into notebook.c is still the wrong way. this should live in search.c with the dialog, for the rest it ought to be transparent - the custom widget is IMO not the right way. There is GtkInfoBar which seems more useful, shares lots of API with dialogs (so it helps maintaining common parts between dialog and bar) and has a less crammed appearance.

@kugel- GtkInfoBar is designed to report important messages to the user. It has bright color, doesn't provide API to add buttons with images, and arrange elements in separate rows. 

After some time everybody gathered their breath, so can we come back to this contentious pull request?