Viewer doesn't show search error messages

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/3720
Reporter	mooffie (@mooffie)

For example, when searching (F7) using an invalid regexp pattern (say "*" or "["), we aren't shown an appropriate error message telling us the pattern is bad. The editor does show such error messages.

(The only "error" message the viewer shows is "Search string not found".)

Note

Original attachments:

• 3720-make-viewer-show-search-error-messages.patch (raw) by mooffie (@mooffie) on Nov 12, 2016 at 21:21 UTC

[mc-butler]

Changed by mooffie (@mooffie) on Nov 5, 2016 at 21:59 UTC (comment 1)

• Owner set to mooffie
• Status changed from new to assigned

[mc-butler]

Changed by mooffie (@mooffie) on Nov 12, 2016 at 21:21 UTC

• Attachment 3720-make-viewer-show-search-error-messages.patch (raw) added

[mc-butler]

Changed by mooffie (@mooffie) on Nov 12, 2016 at 21:22 UTC (comment 2)

NOTE: to understand the patch, you need to read the documentation for mc_search_run()'s return value, in #3693.

[mc-butler]

Changed by mooffie (@mooffie) on Nov 12, 2016 at 21:26 UTC (comment 3)

Explanation for the patch:

The main problem(s) are in mcview_do_search().

mcview_do_search() has the following structure:

{
  boolean found = mcview_find();

  if (orig_search_start != 0 && !found) {
    if (query("Continue from beginning?"))
      search from beginning
  }

  if (!found
        && (view->search->error == MC_SEARCH_E_ABORT
             || view->search->error == MC_SEARCH_E_NOTFOUND))
  {
    ...
    display error message
  }
}

There are two bugs here:

(1) The test view->search->error == MC_SEARCH_E_ABORT || view->search->error == MC_SEARCH_E_NOTFOUND checks for only two of the three possibilities of the failure of mc_search_run(), as documented in the patch at #3693. This test precludes other error codes, like invalid pattern. The fix is simply to remove this test altogether.

(2) We start a "Continue from beginning?"-search even if the failure was because of pattern error or user abort. Instead, we should do this only if the failure was of the MC_SEARCH_E_NOTFOUND kind. Otherwise, besides doing the wrong thing, we also may obliterate a useful error message.

There's also a bug in the backwards search, in mcview_find():

The backwards search, in mcview_find(), works as follows:

while (we're past the beginning) {

  go backwards a bit

  bolean ok = mc_search_run()

  if (ok)
    return TRUE;

  /* Abort search. */
  if (!ok && view->search->error == MC_SEARCH_E_ABORT)
    return FALSE;
}

return FALSE;

(3) The "abort" should occur for any error other than MC_SEARCH_E_NOTFOUND, not just for interactive user abort.

(BTW, a similar bug perhaps exists in the backwards search in the editor. I'll check this out and start a new ticket if that's the case.)

[mc-butler]

Changed by andrew_b (@aborodin) on Nov 14, 2016 at 10:30 UTC (comment 4)

• Branch state changed from no branch to on review
• Status changed from assigned to accepted
• Milestone changed from Future Releases to 4.8.19
• Owner changed from mooffie to andrew_b
• Votes set to andrew_b

Branch: 3720_mcview_search_error_messages
[46f01d70b234a35c2f3f373ffd7ef7f6bce34121]

[mc-butler]

Changed by andrew_b (@aborodin) on Nov 21, 2016 at 8:02 UTC (comment 5)

• Branch state changed from on review to approved

[mc-butler]

Changed by andrew_b (@aborodin) on Nov 21, 2016 at 8:03 UTC (comment 6)

• Branch state changed from approved to merged
• Votes changed from andrew_b to committed-master
• Resolution set to fixed
• Status changed from accepted to testing

Merged to master: [da538f0].

[mc-butler]

Changed by andrew_b (@aborodin) on Nov 21, 2016 at 8:05 UTC (comment 7)

• Status changed from testing to closed