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

 
 

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

Submitted by:  eflister <eflister>
Submitted on:  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

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( 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>
Project 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>
Project 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>
Project 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>
Project Administrator
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>
Project Administrator
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>
Project Administrator
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)
  •  

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

    Only project 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.5