Author Topic: [BUG] Search results toolbar could not keep focused after doing a search  (Read 8648 times)

Ding Zhaojie

  • Senior Community Member
  • Posts: 194
  • Hero Points: 37
I set the search results toolbar docked and auto hide. And I set the Search Options to List all occurrences to Search Results window. After I clicked the "Find" button, the Search Results toolbar only activated for a very short time then it hided immediately, I have to active the Search Results toolbar manually to view the results :(

I checked the code, and I think the problem might in tbfind.e: _show_current_search_window.
Code: [Select]
...
activate_toolbar(window_id.p_active_form.p_name, focus_wid);
...

I printed window_id.p_active_form.p_name, it was empty. So I just changed the code to workaround this problem:
Code: [Select]
activate_toolbar("_tbsearch_form", focus_wid);
Btw: I found in _tbfind_form.ESC() it called _show_current_search_window at the end. But I don't think it is necessary to show the search result when the user cancelled search.

Lee

  • SlickEdit Team Member
  • Senior Community Member
  • *
  • Posts: 1299
  • Hero Points: 130
I will look into case you describe.  The purpose of _show_current_search_window is to restore try and restore focus to previous active window.  If the previous editor was an MDI editor window, it will restore focus there, else if the editor was a control in another Tool window, it will attempt to restore focus to the tool window.  The Find and Replace window is non-modal, so it's possible that it is confused as to where to restore to in some cases.

chrisant

  • Senior Community Member
  • Posts: 1410
  • Hero Points: 131
I used to have my Search Results toolbar docked at the bottom and set to auto-hide.  Since it was auto-hide, I wanted activating the Search Results toolbar to really activate it and set the focus to the appropriate Search<n> tab's editor control.  Instead since it tries to set the focus back to the previous window, what happens every time I started a search was that the Search Results window slid open, and then slid closed again a moment later, and I had to manually open it again so I can see the results.  I'd prefer for it to slide open and get focus so it stays open and I can use the arrow keys to scroll through the results to review them.  It seems to be the same thing that Ding is describing.



On a mildly related note:

When my Search Results toolbar is set to auto-hide then I have the same issue Ding described.  However currently I'm experimenting with having my Search Results toolbar floating, because my Preview toolbar is also docked and set to auto-hide but I want to see previews for search results.  So a few days ago I made a patch to tbsearch.e so that it auto-shows the Preview toolbar, basing it loosely on how open_local_symbol.e auto-shows the Preview toolbar.  Aside from the usual travails about screen real estate this is working pretty nicely.  (The patch auto-shows the Preview toolbar whenever the Search Results toolbar gains focus, as long as they are not in the same tab group).

I think that the Search Results window is designed with the expectation that people will primarily use find-next and find-prev to cycle through the matches.  But I prefer to use the up/down keys to browse the search results and see previews.  So I've concluded that I never want the focus restored to the previous active window when activating the Search Results toolbar.  After all, I can press Esc to put the focus back to the MDI editor window in the rare cases where I don't want to browse the Search Results.  Between changing _show_current_search_window and my auto-show patch, things are working how I like now.  8)

If anyone wants the auto-show patch I can post it.  There are a couple places where existing functions in tbsearch.e need to be modified, but for the most part the patch is just new code (which can even go in a separate file, though I have it embedded in tbsearch.e at the moment).

chrisant

  • Senior Community Member
  • Posts: 1410
  • Hero Points: 131
On another related note:

I tracked down an issue where if the Search Results toolbar is closed and you activate it, the focus gets set to the tab bar instead of to the active search window.  I think it's probably not intended behavior, but I don't know.

In case anyone else ran into that, here is what I found and the patch I made:

This was occurring because activate_search calls _get_active_grep_view to get the search window that should get focus, and then calls activate_toolbar to activate the search toolbar and set focus to the active grep view.  But when the toolbar doesn't actually exist yet, _get_active_grep_view returns '' and the focus doesn't get set properly.  Activating the search toolbar restores the active grep view.  So I patched _get_active_grep_view to optionally first activate the search toolbar so that it can find which tab is active.

tbsearch.e
Code: [Select]
_str _get_active_grep_view(boolean even_if_TB_not_active = false)
{
// ==== BEGIN PATCH ====
   // chrisant: Need to make sure the _tbsearch_form is present before
   // checking to see which tab is active.
   int form_id = _find_object("_tbsearch_form", "n");
   if (!form_id || _tbIsAutoHiddenWid(form_id)) {
      // Check for auto hidden tool window
      form_id = _tbMaybeAutoShow("_tbsearch_form","",true);
      if (!form_id) {
         // This will restore the users last location of the tool window
         tbShow("_tbsearch_form");
      }
   }
// ==== END PATCH ====
   int tab_id = _find_object("_tbsearch_form._search_tab", "n");
   if (tab_id) {
      typeless grep_id;
      int active_id = tab_id._getActiveWindow();
      parse active_id.p_name with "_search_container" grep_id;
      if (grep_id != '') {
         return "_search":+grep_id;
      }
   }
   return ('');
}

tbcmds.e
Code: [Select]
_command void toggle_search()  name_info(','VSARG2_EDITORCTL)
{
   _str focus_ctl = _get_active_grep_view(true);    // PATCH: added true
   _tbToggleTabGroupToolbar('_tbsearch_form',focus_ctl);
}
Code: [Select]
_command void toggle_search()  name_info(','VSARG2_EDITORCTL)
{
   _str focus_ctl = _get_active_grep_view(true);    // PATCH: added true
   _tbToggleTabGroupToolbar('_tbsearch_form',focus_ctl);
}

Ding Zhaojie

  • Senior Community Member
  • Posts: 194
  • Hero Points: 37
I will look into case you describe.  The purpose of _show_current_search_window is to restore try and restore focus to previous active window.  If the previous editor was an MDI editor window, it will restore focus there, else if the editor was a control in another Tool window, it will attempt to restore focus to the tool window.  The Find and Replace window is non-modal, so it's possible that it is confused as to where to restore to in some cases.
Oh I see, I misunderstood the purpose of _show_current_search_window. But I think I don't need to restore the focus back. Here is my final patch to call results after search:

Code: [Select]
static void _show_current_search_window(boolean show_res)
{
   if (show_res) {
       activate_search();
   }
}

Then change the caller like this:

Code: [Select]
_begin_find/_begin_replace:
   ...
   //int current_wid = _get_current_search_wid();
   boolean show_res = (_findlist_all.p_value && _findlist_all.p_enabled);
   ...
   _show_current_search_window(show_res);

_tbfind_form.ESC():
(just comment "_show_current_search_window(current_wid)")

Then the results toolbar will show automatically after search if the search option is set to "List all occurrences".

chrisant

  • Senior Community Member
  • Posts: 1410
  • Hero Points: 131
Ding, you may want to comment out this line in tbfind.e also.

Code: [Select]
int list_all_occurrences(_str search_text, _str search_options, int mfflags, int grep_id)
{
   ...
   //orig_wid._set_focus(); // may have lost focus to search results
   ...
}

Ding Zhaojie

  • Senior Community Member
  • Posts: 194
  • Hero Points: 37
Ding, you may want to comment out this line in tbfind.e also.

Code: [Select]
int list_all_occurrences(_str search_text, _str search_options, int mfflags, int grep_id)
{
   ...
   //orig_wid._set_focus(); // may have lost focus to search results
   ...
}
Thanks, and there is another one in mark_all_occurences().

Graeme

  • Senior Community Member
  • Posts: 2432
  • Hero Points: 322
I was also getting close to reporting the problems with the search results toolbar as described in this thread, so I hope a hotfix can be released that fixes these problems and hopefully it includes Chris's patch to make the preview toolbar show up because I also like to use up/down arrow to preview results  -  this lets me go to the ones I'm interested in much quicker.  I've also been experimenting with floating and non auto-hide and going crazy...

There's also a parallel and almost identical problem with the references toolbar where sometimes it pops up and disappears very quickly and sometimes it stays up but has the focus on the symbol combo box when I want the focus to be on the treeview so I can use up/down arrow to review and/or go quickly to the one I want.  This is with "find references incrementally" turned off.  I haven't figured out what makes the references toolbar sometimes stay up and sometimes not, apart from, of course, when the cursor happens to be in the region of the screen where the toolbar pops up to.  Also, when activate-references is used to show the ref toolbar, it always puts the focus on the symbol combo box which I personally never use  - if I manually activate the references toolbar it's to see the results of the last push-ref command I did.  I know I can patch the code but I much prefer a permanent fix.

Graeme

Lee

  • SlickEdit Team Member
  • Senior Community Member
  • *
  • Posts: 1299
  • Hero Points: 130
I went back through the history to verify and double-checked with older versions of SlickEdit, but the current implementation is historically correct going back to v10, when Find was a modal form.  With the List all Occurences option, the Search Results would get auto-shown (if Auto Hidden), and when the form was dismissed focus returned to the MDI editor window, and the Search Results would be auto-hidden.  When possible we stick with the precedence, but in thinking about it a bit, this is a case where the historical implementation is lacking.

There needs to be new option for List all Occurrences and Find in Files for keyboard focus to switch to the Search Results window or return to the editor window.  I think it does need to be optional.  This new optional behavior would only apply to cases where the Find tool window is auto-dismissed or focus switch is supposed to be automatic.  If the Find tool window is manually closed (ESC or close button), the focus does need to return the active edit window.

Let me pass this on to Scott W. for his review, and I can file the change requests and see if this can possibly be updated in a hotfix.  I also need to review the other code changes posted here to see if they can be incorporated as well.

chrisant

  • Senior Community Member
  • Posts: 1410
  • Hero Points: 131
I've attached my patched tbsearch.e which makes the Search Results toolbar auto-show the Preview toolbar (similar to how open_local_symbol auto-shows the Preview toolbar).

The attached tbsearch.e is from 13.0.1 r17; compare with that version to see the changes.

There are two #defines at the top (with descriptive comments) which when defined enable the two patches.

Graeme

  • Senior Community Member
  • Posts: 2432
  • Hero Points: 322
Quote
Let me pass this on to Scott W. for his review, and I can file the change requests and see if this can possibly be updated in a hotfix.  I also need to review the other code changes posted here to see if they can be incorporated as well.

I have some more comments that hopefully you can think about.

The only tool window I have permanently visible is my own "files" tool window.  I rarely use floating tool windows.  I have nearly all tool windows set to docked, with only some of those set to auto-hide, and in toolbar options I have "auto hide button affects active tab only" enabled/ checked.  I have a strong preference for docked auto-hide rather than docked non auto-hide because with auto-hide, you can dismiss most tool windows by pressing escape. 

With floating tool windows (and sometimes docked as well I think - not sure), whether escape dismisses the window depends which control has the focus. e.g. with search results, escape works only if the "tab control" has the focus, which is pretty inconvenient  - escape should work whichever control has the focus.  My attempts to override escape for the refs tool window didn't work too well.

With docked tool windows, a window can sometimes not get dismissed when it otherwise would be, if the mouse cursor is over the window.  I think the window should be dismissed anyway, in this case, but I realise there might be a technical difficulty implementing this.

The activate-search function attempts to put focus on the "tabs of the tab control".  I would have thought that focus should be on the search results edit window itself, so you can use up/down to pick an item and still use ctrl-tab to switch tabs.

Currently, I use "output to editor window" a lot because I get a big list of results and can select one quickly, but I also use the search results tool window quite often and it's annoying to have to manually activate it if it's set to auto-hide so I would really like it if this could be fixed.

I would also like to have cursor up/down, page up/down in the search results tool window have a "preview capability".  Using the preview tool window for this is not fantastically convenient because I have both preview and search results docked at the bottom of the edit window.  Today, I've tried modifying _grep_cursor (as below) in tbsearch.e to get the main edit window to show the "current search results item" when you cursor up/down in the search results window, but it's not working perfectly yet.  I need to filter out lines that start with "File" (edit - done) and I had to use the p_buf_name assignment (a bad idea) to avoid "flicker" on the search results toolbar, due to the call to "edit" switching focus.  Hence it would be great if this could also be incorporated in a hotfix!  Ideally it should automatically close any files it opened that were not already open without asking too!

I have some problems with the references tool window too but they'll have to wait till another day.  The great thing about the refs tool window is that it has its own preview window.

Graeme

Edit  I imagine a lot of people would like the option to have the preview tool window made visible at the same time as the search results window too, as chrisant has done.

Edit2  another problem with the code below is that the target search line isn't always made visible in the main edit window and centering it in the window isn't necessarily going to show it either.  The only solution I can see for this is to have the target line near the top of the window, and off the top of my head, I'm not sure if this is achievable.


Code: [Select]
static void _grep_cursor()
{
   // now get the destination information and
   // submit it to the preview window
   _str filename;
   int linenum, col;
   int status = _mffindGetDest(filename, linenum, col, true);
   if(status == 0) {
      VS_TAG_BROWSE_INFO cm;
      tag_browse_info_init(cm);
      cm.member_name = "Search result";
      cm.file_name = filename;
      cm.line_no   = linenum;
      cm.column_no = col;

      // added from here
      if (def_grep_cursor_shows_file && linenum > 0) {
         if (!_no_child_windows()) {
            edit(maybe_quote_filename(filename));
            //_mdi.p_child.p_buf_name = filename;  // don't do this
            _mdi.p_child.p_col = col;
            _mdi.p_child.p_line = linenum;
            activate_search();
         }
      }
      // to here

      else {
         cb_refresh_output_tab(cm, true);
      }
   }
}
« Last Edit: July 27, 2008, 01:23:00 pm by Graeme »

hs2

  • Senior Community Member
  • Posts: 2744
  • Hero Points: 288
Hi Graeme, I'm also using the tool windows all docked. I've added this modification to browse the (last) search results in the edit window while having the Preview TB activated. I know it's not perfect and you might need to adopt it a bit, but I'm sure that's easy for you. This also works if the Search TB is not activated. It only should exist of course.
tbsearch.e
Code: [Select]
// HS2-ADD: useCurrent referring to the currently active search tab
_command int grep_last( boolean useCurrent = true ) name_info(','VSARG2_REQUIRES_MDI)
{
   if ( useCurrent )
   {
      _grep_buffer = _get_active_grep_view();
      _grep_buffer = stranslate ( _grep_buffer, '.', '_' );
   }
   ...
}

personal macro toolbox (hs2.e)
Code: [Select]
_command void hs2_grep_last () name_info (','VSARG2_REQUIRES_MDI)
{
   activate_preview();
   grep_last ();
   begin_line ();
}

Maybe it helps a bit, HS2
« Last Edit: July 27, 2008, 10:52:35 am by hs2 »

chrisant

  • Senior Community Member
  • Posts: 1410
  • Hero Points: 131
When I find some time, I plan to patch the Search Results toolbar to have its own built in preview window that can be on the right side, on the bottom side, or hidden.  I'd love to see that feature, maybe the SlickTeam could consider adding it in the future.



Making the Search Results window auto-show the Preview window makes sense at first glance, but there is a fundamental problem:  the Search Results window kind of needs to be large (either tall or wide, but preferably both) to be really useful.  And since I want both to be visible, they can't be in the same dock channel.  On my 1680x1050 screen and on my 1600x1200 screen I simply cannot arrange the SE toolbars in a way to normally allow maximal screen real estate for editing, and show both the Search Results and Preview window only when they're needed.*  But ideally the Search Results window would have its own preview window just like the References window does.  For me, preview is a core feature of grep.  I want to be able to preview no matter how I have my windows arranged.  I also want to be able to maximize my editing area, so I don't have much room for docked-but-not-auto-hidden toolbars.



* It would sort of work if I could arrange the windows like the following, but SE seems to not restore the position/size of the main frame window if the screen resolution is different from last time SE was launched.  I work on machine X at the office using the 1600x1200 monitor attached to it, but I also work on machine X remotely from home using the 1680x1050 monitor built into my laptop.  This confuses SE into not restoring the frame window position/size, so I keep having to manually move/size the SE window to fix the diagram below.  Eventually I'll probably modify the save/restore code to remember position/size for multiple screen resolutions:

Code: [Select]
+-SCREEN-------------------------------------------------------+
|                                                              |
|               +-SE----------------------------------------+  |
| +-GREP------------------+ MENU...                         |  |
| | (floating)            | TOOLBARS...                     |  |
| |                       |-+-------------------------------|  |
| |                       | |                               |  |
| |                       | | MDI editor control            |  |
| |                       | |                               |  |
| |                       | |                               |  |
| |                       | |                               |  |
| |                       | |                               |  |
| |                       | |                               |  |
| +-----------------------+ |                               |  |
|               |           |                               |  |
|               +-------------------------------------------+  |
|               | PREVIEW | (auto-hide/show                 |  |
|               |         |  docked at bottom)              |  |
|               |         |                                 |  |
|               |         |                                 |  |
|               |         |                                 |  |
|               +-------------------------------------------+  |
|                                                              |
+--------------------------------------------------------------+