bugGNU Octave - Bugs: bug #56160, editor: inappropriate auto insert...

 
 

bug #56160: editor: inappropriate auto insert of "end*" when inserting newlines after keyword

Submitter:  eflister <eflister>
Submitted:  Wed 17 Apr 2019 10:32:54 PM UTC
   
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  eflister Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 27 May 2019 04:49:13 PM UTC, comment #10: 

No comments good or bad on the patch so I went ahead and pushed it to the development branch here (https://hg.savannah.gnu.org/hgweb/octave/rev/e14e48e838fa).

Rik <rik5>
Group administrator
Wed 24 Apr 2019 04:52:59 AM UTC, comment #9: 

I attached a patch which seems to fix the cases identified in comment #5.  It doesn't fix the additional problem from comment #7.  This is a hack, but it might be good enough.

(file #46820)

Rik <rik5>
Group administrator
Thu 18 Apr 2019 11:07:13 PM UTC, comment #8: 

Don't have a fix, but I found the problematic code.  The code which automatically closes (adds "endXXX") a keyword is in libgui/src/m-editor/octave-qscintilla.cc.  Quoting it:


void octave_qscintilla::auto_close (int auto_endif, int linenr,
                                    const QString& line, QString& first_word)
{
  // Insert and "end" for an "if" etc., if needed.
  // (Use of "while" allows "return" to skip the rest.
  // It may be clearer to use "if" and "goto",
  // but that violates the coding standards.)

  bool autofill_simple_end = (auto_endif == 2);

  size_t start = line.toStdString ().find_first_not_of (" \t");

  // Check if following line has the same or less indentation
  // Check if the following line does not start with
  //       end* (until) (catch)
  if (linenr < lines () - 1)
    {
      int offset = 1;
      size_t next_start;
      QString next_line;
      do                            // find next non-blank line
        {
          next_line = text (linenr + offset++);
          next_start = next_line.toStdString ().find_first_not_of (" \t\n");
        }
      while (linenr + offset < lines ()
             && next_start == std::string::npos);
      if (next_start == std::string::npos)
        next_start = 0;
      if (next_start > start)       // more indented => don't add "end"
        return;
      if (next_start == start)      // same => check if already is "end"
        {
          QRegExp rx_start = QRegExp (R"((\w+))");
          int tmp = rx_start.indexIn (next_line, start);
          if (tmp != -1 && is_end (rx_start.cap(1), first_word))
            return;
        }
    }

  // If all of the above, insert a new line, with matching indent
  // and either 'end' or 'end...', depending on a flag.

  // If we insert directly after the last line, the "end" is autoindented,
  // so add a dummy line.
  if (linenr + 2 == lines ())
    insertAt (QString ("\n"), linenr + 2, 0);

  // For try/catch/end, fill "end" first, so "catch" is top of undo stack
  if (first_word == "try")
    insertAt (QString (start, ' ')
              + (autofill_simple_end ? "end\n" : "end_try_catch\n"),
              linenr + 2, 0);
  else if (first_word == "unwind_protect")
    insertAt (QString (start, ' ')
              + (autofill_simple_end ? "end\n" : "end_unwind_protect\n"),
              linenr + 2, 0);

  QString next_line;
  if (first_word == "do")
    next_line = "until\n";
  else if (first_word == "try")
    next_line = "catch\n";
  else if (first_word == "unwind_protect")
    next_line = "unwind_protect_cleanup\n";
  else if (autofill_simple_end)
    next_line = "end\n";
  else
    {
      if (first_word == "unwind_protect")
        first_word = '_' + first_word;
      next_line = "end" + first_word + "\n";
    }

  insertAt (QString (start, ' ') + next_line, linenr + 2, 0);
}


When executed, the code searchs downwards from the line containing the keyword looking for a non-blank line.  If this line is at a greater indent level, then itassumes that the block has already been correctly constructed.  So, this is correct


function foo

  x = 1;

endfunction


But if the indent is at the same level as the keyword it is checking then it verifies that the first non-blank line is an end keyword.  This too is correct


function foo

endfunction


If the the indent matches, and it is not an end keyword, then it closes the expression.  So, the indentation of "x = 1;" fools the code in to thinking that there is no "endfunction" and it adds one


function foo

endfunction

x = 1;

endfunction


This is somewhat clever in that the entire file does not need to be parsed, but it can make mistakes as in this case.


Rik <rik5>
Group administrator
Thu 18 Apr 2019 10:52:24 PM UTC, comment #7: 

er, guess i need some markup (any way to edit a prior post?):

  if true
  y=0;
  else
  end

and it gives you this:

  if true
  y=0;
else

  end


eflister <eflister>
Thu 18 Apr 2019 10:49:02 PM UTC, comment #6: 

just noticed that it autoindents incorrectly depending on local indentation as well -- start with this and add a newline after `else`:

  if true
  y=0;
  else
  end

and it gives you this:

  if true
  y=0;
else
 
  end

eflister <eflister>
Thu 18 Apr 2019 06:44:47 PM UTC, comment #5: 

Ok, confirmed. This example shows the same result


function foo

x = 1;

endfunction


Or this example


if (x)
y = 0;
endif


Pressing Enter at the end of the first line in each example adds a new 'endfunction' or 'endif'. The detection of matching 'end' keywords seems to be dependent on having proper indentation.

Mike Miller <mtmiller>
Group Member
Thu 18 Apr 2019 06:38:47 PM UTC, comment #4: 

sorry, just verified my instructional comment interfered.  remove comments and then test.

eflister <eflister>
Thu 18 Apr 2019 06:36:34 PM UTC, comment #3: 

Here's what I have in the editor before


function % <-- place cursor here and hit return
x=1; % no indent
endfunction


I move the cursor to the left of the '%' character on the first line, press Enter, and this is what I have in the editor after


function
  % <-- place cursor here and hit return
x=1; % no indent
endfunction


Mike Miller <mtmiller>
Group Member
Thu 18 Apr 2019 06:29:11 PM UTC, comment #2: 

ah tracked it down to only the case that matlab-style "zero first-level indenting" is used inside a function, ie:

function % <-- place cursor here and hit return
x=1; % no indent
endfunction

eflister <eflister>
Thu 18 Apr 2019 05:59:36 PM UTC, comment #1: 

Can you give an example of how to demonstrate this in the editor? When I try to reproduce this I just get blank lines inserted, no spurious endif or endfunction keywords added.

Mike Miller <mtmiller>
Group Member
Wed 17 Apr 2019 10:32:54 PM UTC, original submission:  

when editing an already syntactically balanced keyword/end pair (function/endfunction, etc), by inserting newlines after the keyword, the auto insert function is adding inappropriate 'end*' lines that have to be manually removed.  detect whether there is already a balanced matching end so you can suppress the auto insert.

eflister <eflister>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #46820:  56160.cset added by rik5 (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by eflister (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-05-28 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2019-05-27 rik5 Release5.1.0 dev
        Operating SystemMac OS Any
    2019-04-24 rik5 Attached File- Added 56160.cset, #46820
        StatusConfirmed Patch Submitted
    2019-04-18 mtmiller StatusNeed Info Confirmed
    2019-04-18 mtmiller StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code