bugGNU Octave - Bugs: bug #58372, Editor: Highlight all occurrences...

 
 

bug #58372: Editor: Highlight all occurrences of word selected by double click centers text in editor window when off screen

Submitter:  Dan Sebald <sebald>
Submitted:  Fri 15 May 2020 04:27:41 AM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  sebald Open/Closed:  * Closed
Release:  * 5.1.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Sat 16 May 2020 10:05:00 AM UTC, comment #2: 

Dan, thank you for the report and the patch. The latter is pushed as changeset http://hg.savannah.gnu.org/hgweb/octave/rev/1127bcb30e61

Closing the report.

Torsten Lilge <ttl>
Group Member
Sat 16 May 2020 08:01:51 AM UTC, comment #1: 

There is no proper QScintilla method like getXOffset() and setxOffset().  However, it's possible to access the proper routines via the base class's SendScintilla() method.

Patch is attached.  Please consider applying prior to the upcoming version 6.1 release.

I've also attached a screen shot showing how the view remains the same.  Very nice in the sense that there is a green dash in the left column to indicate there is an occurrence on the line even though the word isn't visible because it extends outside the area.

(file #49087,

Dan Sebald <sebald>
Fri 15 May 2020 04:27:41 AM UTC, original submission:  

I came across the following subtle behavior of the "Highlight all occurrences of word selected by double click" which is quite annoying pretty much obviating an otherwise useful feature.

When there are multiple occurrences of a comment word, variable name, function name in a file and one of the occurrences extends past the end of the visible area, double clicking the word in the visible part of the area will center the text on screen.  That is not only annoying, but it also has the side effect of selecting more text because of the repositioning of the text area.

Here is some sample text to copy into the editor:

for i=1:50
  input1 = sqrt(pi);
  x = do_not_make_your_function_names_too_long_because_otherwise_they_go_off_the_end_of_the_editor_screen(input1, input2);
  input2 = x / input1;
endfor

Make sure that the editor is narrow enough so that the first "input1" and "input2" go outside the visible area.  Now double-click "input2".

Attached is what I'm seeing.  Notice how what I clicked on "input2" has shifted off the screen and with that shift the " = x " is additionally highlighted.  Basically, with "Highlight all occurrences" turned on, one can't double click a word and paste with what is copied in the clipboard.

This seems like one of those "seemed like a good idea at the time" sorts of things, but with an unexpected consequence.

The code where the action is happening is in file-editor-tab.cc:

        if (m_highlight_all_occurrences)
          {
            // Clear any previous selection.
            m_edit_area->set_word_selection ();

            // highlighting of all occurrences of the clicked word is enabled

            // get the resulting cursor position
            // (required if click was beyond a line ending)
            int line, col;
            m_edit_area->getCursorPosition (&line, &col);

            // get the word at the cursor (if any)
            QString word = m_edit_area->wordAtLineIndex (line, col);
            word = word.trimmed ();

            if (! word.isEmpty ())
              {
                // word is not empty, so find all occurrences of the word

                // remember first visible line for restoring the view afterwards
                int first_line = m_edit_area->firstVisibleLine ();

                // search for first occurrence of the detected word
                bool find_result_available
                  = m_edit_area->findFirst (word,
                                            false,   // no regexp
                                            true,    // case sensitive
                                            true,    // whole words only
                                            false,   // do not wrap
                                            true,    // forward
                                            0,0,     // from the beginning
                                            false
#if defined (HAVE_QSCI_VERSION_2_6_0)
                                            , true
#endif
                                           );

                // loop over all occurrences and set the related indicator
                int oline, ocol;
                int wlen = word.length ();

                while (find_result_available)
                  {
                    // get cursor position after having found an occurrence
                    m_edit_area->getCursorPosition (&oline, &ocol);
                    // mark the selection
                    m_edit_area->show_selection_markers (oline, ocol-wlen, oline, ocol);

                    // find next occurrence
                    find_result_available = m_edit_area->findNext ();
                  }

                // restore the visible area of the file, the cursor position,
                // and the selection
                m_edit_area->setFirstVisibleLine (first_line);
                m_edit_area->setCursorPosition (line, col);
                m_edit_area->setSelection (line, col - wlen, line, col);
                m_edit_area->set_word_selection (word);
              }
          }


I'm not sure, but looking at those last few lines, it might be the action of looping through all the finds of the word highlighting them might be what's inherently moving the text area.  Then the visible area is restored the way things were before the highlight all was run.  It looks as though there needs to be another area modification which is to return to the first visible column before the search, something analogous to the

m_edit_area->setFirstVisibleLine (first_line);

Here is the Scintilla documentation:

SCI_SETFIRSTVISIBLELINE(line displayLine)
SCI_GETFIRSTVISIBLELINE → line
These messages retrieve and set the line number of the first visible line in the Scintilla view. The first line in the document is numbered 0. The value is a visible line rather than a document line.

SCI_SETXOFFSET(int xOffset)
SCI_GETXOFFSET → int
The xOffset is the horizontal scroll position in pixels of the start of the text view. A value of 0 is the normal position with the first text column visible at the left of the view.

It is probably getXOffset() and setXOffset() that we need to record and restore.

Dan Sebald <sebald>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by sebald (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.

    Only group members can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-05-16 ttl StatusNone Fixed
        Open/ClosedOpen Closed
    2020-05-16 sebald Attached File- Added octave-highlight_all_occurrences_x_offset-djs2020may15.patch, #49087
        Attached File- Added Screenshot@from@2020-05-16@03-55-06_editor_after_double_click_input2_fix-CROPPED.png, #49088
    2020-05-15 sebald Attached File- Added Screenshot@from@2020-05-15@00-01-19_editor_after_double_click_input2_CROPPED.png, #49068

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code