patch for completion in octave

patch for completion in octave

[Alexander Klimov]

Hi.

The current version of inferior-octave-complete includes in the string
for completion symbols like `=', e.g., after

 M-x run-octave

 x=lins<Tab>

user gets an error

 No completions of x=linsp

Since in octave

 The name of a variable must be a sequence of letters, digits and
 underscores,

the following patch corrects the problem:

=== modified file 'lisp/progmodes/octave-inf.el'
--- lisp/progmodes/octave-inf.el        2011-01-26 08:36:39 +0000
+++ lisp/progmodes/octave-inf.el        2011-02-08 12:45:02 +0000
@@ -267,7 +267,7 @@
   (let* ((end (point))
         (command
          (save-excursion
-           (skip-syntax-backward "w_" (comint-line-beginning-position))
+           (skip-chars-backward "a-zA-Z0-9_" (comint-line-beginning-position))
            (buffer-substring-no-properties (point) end)))
         (proc (get-buffer-process inferior-octave-buffer)))
     (cond (inferior-octave-complete-impossible

-- 
Regards,
ASK

Re: patch for completion in octave

[Stefan Monnier]

> The current version of inferior-octave-complete includes in the string
> for completion symbols like `=', e.g., after

>  M-x run-octave

>  x=lins<Tab>

> user gets an error

>  No completions of x=linsp

> Since in octave

>  The name of a variable must be a sequence of letters, digits and
>  underscores,

> the following patch corrects the problem:

> === modified file 'lisp/progmodes/octave-inf.el'
> --- lisp/progmodes/octave-inf.el        2011-01-26 08:36:39 +0000
> +++ lisp/progmodes/octave-inf.el        2011-02-08 12:45:02 +0000
> @@ -267,7 +267,7 @@
>    (let* ((end (point))
>          (command
>           (save-excursion
> -           (skip-syntax-backward "w_" (comint-line-beginning-position))
> +           (skip-chars-backward "a-zA-Z0-9_" (comint-line-beginning-position))
>             (buffer-substring-no-properties (point) end)))
>          (proc (get-buffer-process inferior-octave-buffer)))
>      (cond (inferior-octave-complete-impossible

Thanks for your report.  Before going ahead and installing your patch,
I'd like to make sure I understand the problem: as far as I can tell,
the character = has syntax "." in octave-mode, but for some reason
octave-inf.el does not use octave-mode's syntax-table.

So, could you try the patch below instead for a little while and see if
it fixes your problem (it should) and if it doesn't introduce
other issues?


        Stefan


=== modified file 'lisp/progmodes/octave-inf.el'
--- lisp/progmodes/octave-inf.el	2011-01-26 08:36:39 +0000
+++ lisp/progmodes/octave-inf.el	2011-02-08 20:38:06 +0000
@@ -73,10 +73,7 @@
   "Keymap used in Inferior Octave mode.")
 
 (defvar inferior-octave-mode-syntax-table
-  (let ((table (make-syntax-table)))
-    (modify-syntax-entry ?\` "w" table)
-    (modify-syntax-entry ?\# "<" table)
-    (modify-syntax-entry ?\n ">" table)
+  (let ((table (make-syntax-table octave-mode-syntax-table)))
     table)
   "Syntax table in use in inferior-octave-mode buffers.")
 

Re: patch for completion in octave

[Alexander Klimov]

Hi.

On Tue, 8 Feb 2011, Stefan Monnier wrote:
> So, could you try the patch below instead for a little while and see if
> it fixes your problem (it should) and if it doesn't introduce
> other issues?

Your patch is also a good idea since it removes a code duplication,
but besides fixing the original problem, it evinces that we have

    (modify-syntax-entry ?. "w"   table)
    (modify-syntax-entry ?_ "w"   table)

in octave-mod.el, that makes M-f or M-b on something like

 var.field.subfield_1

too greedy.

I must admit that I am too new to octave-mode, so I suspect I am not
qualified to test the changes.

Besides the original problem with completion, during testing I noticed
that octave-help is not a command (btw, would not it be a good idea
for define-key to warn that its argument is not a command, instead of
reporting an error when the key is pressed?).

In the menu "Lookup Octave Index" does not work since info-lookup-mode
is not set (maybe we should set lookup mode "globally" instead and
remove octave-help?).

There is also a problem with completion-addsuffix, since `v<Tab>'
gives `var ' and now the user has to press DEL to continue with
`.field' (btw, since the structure completion is possible, I added `.'
into the skip pattern below).

Together with your patch the following is supposed to fix all the
problems (but, again, I doubt the quality of my testing):

=== modified file 'lisp/progmodes/octave-inf.el'
--- lisp/progmodes/octave-inf.el        2011-01-26 08:36:39 +0000
+++ lisp/progmodes/octave-inf.el        2011-02-09 13:20:42 +0000
@@ -267,7 +264,7 @@
   (let* ((end (point))
         (command
          (save-excursion
-           (skip-syntax-backward "w_" (comint-line-beginning-position))
+           (skip-chars-backward "a-zA-Z_." (comint-line-beginning-position))
            (buffer-substring-no-properties (point) end)))
         (proc (get-buffer-process inferior-octave-buffer)))
     (cond (inferior-octave-complete-impossible
@@ -290,9 +287,10 @@
                   (setcdr x (setq y (cdr y)))
                 (setq x y
                       y (cdr y)))))
-          ;; And let comint handle the rest
-          (comint-dynamic-simple-complete
-           command inferior-octave-output-list)))))
+          ;; And let comint handle the rest (without adding a suffix)
+          (let (comint-completion-addsuffix)
+            (comint-dynamic-simple-complete
+             command inferior-octave-output-list))))))

 (defun inferior-octave-dynamic-list-input-ring ()
   "List the buffer's input history in a help buffer."

=== modified file 'lisp/progmodes/octave-mod.el'
--- lisp/progmodes/octave-mod.el        2011-01-25 04:08:28 +0000
+++ lisp/progmodes/octave-mod.el        2011-02-09 13:03:07 +0000
@@ -279,7 +279,7 @@
     ["Submit Bug Report"	octave-submit-bug-report t]
     "-"
     ["Describe Octave Mode"	describe-mode t]
-    ["Lookup Octave Index"	info-lookup-symbol t]))
+    ["Lookup Octave Index"	octave-help t]))

 (defvar octave-mode-syntax-table
   (let ((table (make-syntax-table)))
@@ -299,8 +299,8 @@
     ;; Was "w" for abbrevs, but now that it's not necessary any more,
     (modify-syntax-entry ?\` "."  table)
     (modify-syntax-entry ?\" "\"" table)
-    (modify-syntax-entry ?. "w"   table)
-    (modify-syntax-entry ?_ "w"   table)
+    (modify-syntax-entry ?. "."   table)
+    (modify-syntax-entry ?_ "."   table)
     ;; The "b" flag only applies to the second letter of the comstart
     ;; and the first letter of the comend, i.e. the "4b" below is ineffective.
     ;; If we try to put `b' on the single-line comments, we get a similar
@@ -661,6 +661,7 @@
 (defun octave-help ()
   "Get help on Octave symbols from the Octave info files.
 Look up symbol in the function, operator and variable indices of the info files."
+  (interactive)
   (let ((info-lookup-mode 'octave-mode))
     (call-interactively 'info-lookup-symbol)))


-- 
Regards,
ASK

Re: patch for completion in octave

[Stefan Monnier]

>> So, could you try the patch below instead for a little while and see if
>> it fixes your problem (it should) and if it doesn't introduce
>> other issues?

> Your patch is also a good idea since it removes a code duplication,
> but besides fixing the original problem, it evinces that we have

>     (modify-syntax-entry ?. "w"   table)
>     (modify-syntax-entry ?_ "w"   table)

> in octave-mod.el, that makes M-f or M-b on something like

I've installed a patch which should fix this, along with the poriginal
completion problem in octave-inf.

> In the menu "Lookup Octave Index" does not work since info-lookup-mode
> is not set (maybe we should set lookup mode "globally" instead and
> remove octave-help?).

I've set info-lookup-mode in inferior-actave-mode indeed, so that the
standard binding for info-lookup-symbol (C-h S) works.

> There is also a problem with completion-addsuffix, since `v<Tab>'
> gives `var ' and now the user has to press DEL to continue with
> `.field' (btw, since the structure completion is possible, I added `.'
> into the skip pattern below).

The new code does not use comint's comint-dynamic-simple-complete but
also gets rid of the addsuffix.  This may not always be the right thing
to do (it's sometimes convenient to have the SPC suffix), but until we
can determine when it's good and when it's not, I'd rather not add any.


        Stefan