bugGNU TeXmacs - Bugs: bug #33131, "Search next" action is...

 
 

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

bug #33131: "Search next" action is not in the menus and the shortcut is hidden

Submitted by:  Lebreton Romain <lebreton>
Submitted on:  Wed 20 Apr 2011 12:01:18 PM UTC  
 
Category: User InterfacePriority: 5 - Normal
Item Group: ErrorStatus: Wont Fix
Privacy: PublicAssigned to: Joris van der Hoeven <vdhoeven>
Originator Name: Open/Closed: Closed
Release: 10.7.10Release: None
Fixed Release: NoneFixed Release: 
Keywords: 

(Jump to the original submission Jump to the original submission)

Fri 23 Mar 2012 07:26:49 AM UTC, comment #22:

The search-tree function as it is now should be regarded as deprecated. I would very much prefer that you do not touch this code or rely on this code. It will be replaced as soon as all search/replace/spell-check features will be based on TeXmacs input widgets on a separate footer bar. Please let us end this discussion now. I put everything back in a reasonable state and would like to stop losing time with this issue.

Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Thu 22 Mar 2012 11:16:38 PM UTC, comment #21:

I intentionally left the search-tree function in the code while reverting, but I see you removed it in rev 5173 "Fixes". I rely on this function elsewhere (the code is not in the svn). If I want to be able to retrieve the search tree, how I am supposed to do it? Is there another way I am not aware of?

Miguel de Benito <mdbenito>
Project Member
Thu 22 Mar 2012 03:42:07 PM UTC, comment #20:

I already replied you: I am following the Emacs conventions. Also it is possible to search the same string again by pressing C-s (or the system-dependent shortcut) twice. So thanks for reverting.

Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Thu 22 Mar 2012 02:26:25 PM UTC, comment #19:

I'm sorry, but you have to realize how inadequate "not OK" or "wrong with the current conventions" is as an answer to the question "what's wrong with this idea?". What's "not ok"? What are those conventions? Are they for some reason immutable?

But since this conversation is bordering on the absurd and, even though you don't want to share them, you certainly have your reasons not to want to let people search while not in search mode, I will undo the changes to the keybindings and the menus in the hope that that's what you mean with: "the main part of your patch".

Miguel de Benito <mdbenito>
Project Member
Wed 21 Mar 2012 02:25:35 PM UTC, comment #18:

For your information: the behaviour before your patch was as for Emacs. A better approach now that we support TeXmacs input widgets would be a separate search and replace bar with buttons. But that is for later. __Please__ revert your patch; it is wrong with the current conventions.

Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Wed 21 Mar 2012 02:02:10 PM UTC, comment #17:

You may keep the change in the menus, but please undo the rest of the patch. It is not OK.

Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Wed 21 Mar 2012 11:23:44 AM UTC, comment #16:

Hi, sorry for the delay.

I'm reading the conversation below and I still don't see why it is wrong to be able to resume the search without going into search mode: what I question is the need for the shortcuts to work only in search mode. This is totally non-standard and counter-intuitive

Are there any technical grounds why this has to be so? I see none in the code and the feature works now as it is (although under MacOS there's the problem with case sensitivity in shortcuts, but that's another story), but please, explain.

Undoing "the main part of my patch" would mean disabling the extra menu items and revert the shortcuts to what I consider their less than optimal configuration. How is this better?

Miguel de Benito <mdbenito>
Project Member
Wed 14 Mar 2012 10:23:13 PM UTC, comment #15:
  • Please undo the main part of your patch (the search prefix was used for making the keystrokes relative to searching mode).
  • For the menus, you may change "Search" into "Search next" when we are already in search mode.
Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Wed 14 Mar 2012 04:14:09 PM UTC, comment #14:

No prob. I'll revert. However are you sure you have read what the patch does? Your answer again seems to indicate not. You say:

Please revert your patch and also your change to make "Find next" only appear with detailed-menus?

No error was fixed. Merely two items for "search next" and "search previous" added to the edit menu, under the condition that detailed-menus? was #t.

Then you add:

Under windows pressing twice C-f is not assumed to work

Who said it should? I didn't (advertently) implement that.

So there was nothing to be corrected at the first place.

Well, the items were not there in the menus, how can the user know what the key is?

Also the new (I fixed nothing) function in the glue returns the current search tree. Something which in my opinion could be useful to repopulate a search widget with the previous search(es)

Miguel de Benito <mdbenito>
Project Member
Wed 14 Mar 2012 08:33:06 AM UTC, comment #13:

No, you misunderstood me. I am sorry for not being sufficiently clear. Please revert your patch and also your change to make "Find next" only appear with detailed-menus?

There actually was no bug at all in the original code. Under windows pressing twice C-f is not assumed to work (at least not when following the standard guidelines for windows keyboard shortcuts). C-f is for search and one should use C-g or F3 for "search next".

So there was nothing to be corrected at the first place.
I just added two shortcuts so that C-f C-f and C-f C-F will always work now. This should not break anything anyway. So please revert evrything you did, because it may introduce various other subtle bugs and is inconsistent with the way I now deal with keyboard shortcuts in special modes (search, spell, replace, ...).

Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Wed 14 Mar 2012 02:17:41 AM UTC, comment #12:

I'm assuming that "I looked at the code and everything is correct, strictly speaking." means you're ok with the patch, and I'm reading "For convenience I would add" where you wrote "For convenience I added", so I'm committing the stuff as it was. Please correct me if I'm wrong.

Also reopen if necessary.

Miguel de Benito <mdbenito>
Project Member
Fri 09 Mar 2012 09:00:45 PM UTC, comment #11:

By the way, I changed the "Find next" options to appear only with detailed-menus? true.

Miguel de Benito <mdbenito>
Project Member
Fri 09 Mar 2012 08:59:44 PM UTC, comment #10:

So you applied the patch or found the idea bad? It's not clear from what you say.

Miguel de Benito <mdbenito>
Project Member
Fri 09 Mar 2012 08:45:08 PM UTC, comment #9:

I added the standard keyboard shortcuts for your OS.
I looked at the code and everything is correct, strictly speaking.
For convenience I added additional bindings for C-f and C-F
in search mode, but they are really redundant and non standard.

Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Fri 09 Mar 2012 08:44:53 PM UTC, comment #8:

I added the standard keyboard shortcuts for your OS.
I looked at the code and everything is correct, strictly speaking.
For convenience I added additional bindings for C-f and C-F
in search mode, but they are really redundant and non standard.

Joris van der Hoeven <vdhoeven>
Project AdministratorIn charge of this item.
Fri 09 Mar 2012 08:28:52 PM UTC, comment #7:

Thanks Philippe for the hint.

Because the tree of what has been searched is not deleted until a new search is performed, I've added a function to the glue wich returns it. Using that we can activate new menu items to search-next or previous.

I've also modified the kbd-map for macos, win, gnome and kde to what is the default behaviour in those platforms: "search again" is active as soon as some string was searched.

You have a preliminary version provided in the new patch attached.

(file #25311)

Miguel de Benito <mdbenito>
Project Member
Fri 09 Mar 2012 06:33:34 PM UTC, comment #6:

The following comment is not about fixing the visibility of the shortcut, but about not beeing able to resume searching, which came up in the discussion.

Looking into the code I saw that everything is indeed setup so that a search can resume after quitting the search mode. You can try it by entering the Scheme command (search-next) or (search-previous) in Tool->Execute->Evaluate Scheme expression. (note that you do not really go back to search mode: you can no longer change the searched string).

The only thing missing for the user to use this feature is a menu command or a keyboard shortcut. In linux for instance F3 (search next) is only active in search mode, but it is not used outside of search mode. So if you add to kbd-map

("F3" (search-next))
("S-F3" (search-previous))

in the Gnome keymap section of generic-kdb.scm, the F3 and Shit-F3 shortcuts will also work outside of search mode. I haven't seen any side effect. Of course it applies also to other look-and-feel with different shortcuts.

Philippe Joyez <pjoyez>
Project Member
Sat 16 Jul 2011 08:56:48 AM UTC, comment #5:

For me the main benefit of the menu item isn't that it works, but that it displays the short-cut key for all to see.

It should therefore not be a nested menu.

(Of course not-working is also a bug, but a much less urgent one).

However I note your comments to the -dev mailing list about the short-cut key also not showing unless search mode is enabled.

It is a point of annoyance for me that search mode is so easily exited, and I wonder if in non-search mode, the search-next key should repeat the last search. It annoys me that I current have to re-enter the same search string.

The patch you posted works for me, but does not show the shortcut key, which I see you note on your mail to the -dev list.

The hacky fix is to allow the search-next key to also work in non-search mode and cause it to invoke the search mode if it is not already enabled.

I don't think this would break user expectation - as I say, and you note, search mode ends too easily. Also in the users mind, I don't think search-mode is really a different mode - in most editors find-next ALWAYS works.

Sam Liddicott <samjam>
Project Member
Fri 15 Jul 2011 09:35:36 PM UTC, comment #4:

Try the patch for a solution. An improvement on this could make it into the code.

(file #23659)

Miguel de Benito <mdbenito>
Project Member
Fri 15 Jul 2011 07:21:49 PM UTC, comment #3:

You are right indeed... Not everybody is going to find generic-kbd.scm, or to want to look for it in the first place, for that matter. Proposals? Add menu items seems natural, but as nested menus or better only shown during a search?

Miguel de Benito <mdbenito>
Project Member
Thu 14 Jul 2011 09:26:24 PM UTC, comment #2:

We should note close the bug so hastily.

The actual bug is that it is nearly impossible to find out what the "search-next" key is and that there is no search-next on the menus.

The feature may "work" but it is hidden

Sam Liddicott <samjam>
Project Member
Fri 06 May 2011 02:49:31 PM UTC, comment #1:

In default look-and-feel for linux, F3 key does the search-next operation. I had to search around the source files to discover this

Sam Liddicott <samjam>
Project Member
Wed 20 Apr 2011 12:01:18 PM UTC, original submission:

This is only valid when the "look and feel" option is set to default or windows. The "search" shortcut is then Ctrl+f. It finds the first occurence. But if you type Ctrl+f again, it does not go to the next occurence.
The problem is not present in the emacs look and feel option.

Lebreton Romain <lebreton>
Project Member

 

Attached Files
file #25311:  search-patch-rev4962.diff added by mdbenito (5KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by vdhoeven (Posted a comment)
  • -unavailable- added by pjoyez (Posted a comment)
  • -unavailable- added by mdbenito (Updated the item)
  • -unavailable- added by samjam (Posted a comment)
  • -unavailable- added by lebreton (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 25 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 23 Mar 2012 07:26:49 AM UTCvdhoevenAssigned tomdbenito=>vdhoeven
    Thu 22 Mar 2012 09:35:59 PM UTCmdbenitoStatusNeed Info=>Wont Fix
      Open/ClosedAccepted=>Closed
    Thu 22 Mar 2012 02:26:25 PM UTCmdbenitoPriority7 - High=>5 - Normal
      Open/ClosedOpen=>Accepted
    Wed 21 Mar 2012 11:23:44 AM UTCmdbenitoPriority9 - Immediate=>7 - High
    Wed 14 Mar 2012 04:14:09 PM UTCmdbenitoStatusConfirmed=>Need Info
    Wed 14 Mar 2012 08:33:06 AM UTCvdhoevenPriority4 - Bit lower=>9 - Immediate
      StatusFixed=>Confirmed
      Open/ClosedClosed=>Open
    Wed 14 Mar 2012 02:17:41 AM UTCmdbenitoOpen/ClosedAccepted=>Closed
    Wed 14 Mar 2012 02:17:40 AM UTCmdbenitoStatusNeed Info=>Fixed
    Fri 09 Mar 2012 09:14:49 PM UTCmdbenitoOpen/ClosedClosed=>Accepted
    Fri 09 Mar 2012 09:14:48 PM UTCmdbenitoStatusFixed=>Need Info
    Fri 09 Mar 2012 08:45:08 PM UTCvdhoevenStatusReady For Test=>Fixed
    Fri 09 Mar 2012 08:44:53 PM UTCvdhoevenOpen/ClosedOpen=>Closed
    Fri 09 Mar 2012 08:29:03 PM UTCmdbenitoAttached File#23659=>Removed
    Fri 09 Mar 2012 08:28:52 PM UTCmdbenitoAttached File-=>Added search-patch-rev4962.diff, #25311
    Fri 15 Jul 2011 09:35:36 PM UTCmdbenitoAttached File-=>Added Diff edit-menu.scm.patch, #23659
      StatusIn Progress=>Ready For Test
    Fri 15 Jul 2011 07:21:49 PM UTCmdbenitoPriority5 - Normal=>4 - Bit lower
      StatusWorks For Me=>In Progress
      Assigned toNone=>mdbenito
      Open/ClosedClosed=>Open
      Release=>10.7.10

    Back to the top


    Powered by Savane 3.1-cleanup1