patchGNU nano - Patches: patch #9434, browser: allow backward/forward...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9434: browser: allow backward/forward researches the same way the help viewer does

Submitter:  David Lawrence Ramsey <dolorous>
Submitted:  Mon 14 Aug 2017 09:38:12 PM UTC
   
 
Priority:  3 - Low Status:  Done
Privacy:  Public Assigned to:  dolorous
Open/Closed:  Closed Release:  None

Jump to the original submission

Sun 20 Aug 2017 07:31:33 AM UTC, comment #10: 

Oops.  I had it in my repo, but forgot to git add and commit --amend it.  Sorry.  Pushed now, a56a881c.

Benno Schulenberg <bens>
Group administrator
Sat 19 Aug 2017 10:08:30 PM UTC, comment #9: 

No problem.  However, when you inverted the boolean, you didn't do so for the do_fileresearch() calls, so Alt-Up now re-searches forward, while Alt-Down and Alt-W re-search backward.

The attached patch against git 633343d fixes this.

(file #41582)

David Lawrence Ramsey <dolorous>
Group Member
Sat 19 Aug 2017 08:01:45 PM UTC, comment #8: 

Pushed to git, 633343de, with an inversion of the boolean.  Thanks for the patch.

Benno Schulenberg <bens>
Group administrator
Sat 19 Aug 2017 07:26:25 PM UTC, comment #7: 

Attached new version that builds properly with --enable-tiny --enable-browser (modeled after the similar fix for the help browser), synced with git 5fcda55.

(file #41580)

David Lawrence Ramsey <dolorous>
Group Member
Tue 15 Aug 2017 08:58:41 PM UTC, comment #6: 

Okay.  Although if functionality is there, it should still work properly.  And good to know on Meta-W.

I've attached a new version that I think should be more to your liking.  In it, there's also a fix for an off-by-one error in backwards search that I found after more testing: "if (looking_at-- == 0)" is actually the correct code, since 0 is the first element; without that, searching for ".." in the source directory of nano (which there should be only one entry for) and then using Alt-Up to search again would segfault because looking_at would go out of range in that case.

For the record, however, I don't like the idea of hidden features very much.  One of the main reasons I started using this editor in the first place was because all the features were documented in the help and easily accessible.

(file #41533)

David Lawrence Ramsey <dolorous>
Group Member
Tue 15 Aug 2017 07:48:18 PM UTC, comment #5: 

Inconsistency is in this case a small problem.  I can still imagine people searching a help text for something like
"next" or "word" or "line" -- things that will occur multiple times and that the user might want to flip back and forth between.  But in the file browswer...  File names try to be unique, and if the user remembers a fragment of the name, chances are that it is the only occurrence.  Besides, I think very few people use the file browser, and far less use its search function.

(It is a small problem that on an 80-column terminal the M-W shortcut isn't shown in the help lines.  I'll see if I can change that.  It may not look nice, to have a ^W and M-W, which are of different widths, in one column.)

Anyway, if you want to reduce the amount of code, you can drop the add_to_funcs() calls -- the feature will be "hidden" (just like Q and / and N and n).  And don't add unnecessary comments, like in the first hunk for browser.c.  Don't rename do_fileresearch(), by the way.  And you can elide another four lines by doing something like: "if (--looking_at == 0)...".

Benno Schulenberg <bens>
Group administrator
Tue 15 Aug 2017 04:23:45 PM UTC, comment #4: 

Addendum: Since comment #1 doesn't actually have the link to the patch, I mean file #41523 specifically.

David Lawrence Ramsey <dolorous>
Group Member
Tue 15 Aug 2017 03:56:47 PM UTC, comment #3: 

Inconsistency between interfaces is still a problem, though, which is why I did.

As for the problem, leaving BACKWARDS_SEARCH unset is what the first version of the patch, in comment #1, does, so that's covered.  It's a bit tricky to figure out.

As for "too much code", the main addition in terms of lines is the code in findfile() (formerly findnextfile()) to move through the list backwards, but that's not something that can be left out if you want this to work.  And moving it to a separate function might remove the duplicated "Search Wrapped" line, but might end up adding more lines in total due to having to pass in a variable reference in order to change it later.  What would you have me do to fix this?

David Lawrence Ramsey <dolorous>
Group Member
Tue 15 Aug 2017 09:54:36 AM UTC, comment #2: 

Too much code.  That is the reason I didn't add Alt+Up/Down to the file browser.

Also, the patch is incorrect.  When I search, for example, for "b", Alt+Down will find each occurrence, while Alt+Up will find only the leading bees.  The patch should leave BACKWARDS_SEARCH unset, as it was.

Benno Schulenberg <bens>
Group administrator
Mon 14 Aug 2017 09:56:12 PM UTC, comment #1: 

Argh.  Attached new version, that properly does the search for text within the filenames (in order to find matches) backwards, as well as the search for the filename itself backwards.

(file #41524)

David Lawrence Ramsey <dolorous>
Group Member
Mon 14 Aug 2017 09:38:12 PM UTC, original submission:  

Currently, as of git 566a2da, the help browser can search backwards and forwards for a previous search string using Alt-Up and Alt-Down (or N and n).  However, the file browser cannot do this (and still treats both N and n as keys for forward searches, as the help browser used to, which is inconsistent with the help browser now).

The attached patch adds this functionality to the file browser.

David Lawrence Ramsey <dolorous>
Group Member

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by dolorous (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-28 bens Open/ClosedOpen Closed
    2017-08-19 dolorous Attached File- Added 0001-browser-fix-inverted-search-direction-when-calling-d.patch, #41582
    2017-08-19 bens StatusNone Done
        Assigned toNone dolorous
    2017-08-19 dolorous Attached File- Added 0001-browser-allow-backward-forward-searches-with-Alt-Up-.patch, #41580
    2017-08-15 dolorous Attached File- Added 0001-browser-allow-backward-forward-searches-with-Alt-Up-.patch, #41533
    2017-08-15 bens Summarybrowser: allow backward/forward researches the same way the help browser does browser: allow backward/forward researches the same way the help viewer does
    2017-08-15 bens Priority5 - Normal 3 - Low
    2017-08-14 dolorous Attached File- Added 0001-browser-allow-backward-forward-searches-with-Alt-Up-.patch, #41524
    2017-08-14 dolorous Attached File- Added 0001-browser-allow-backward-forward-searches-with-Alt-Up-.patch, #41523

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code