bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Rolf Ade]

Recipt:

- Start emacs -Q

- Open some buffer and enable tcl-mode (M-x tcl-mode).

Copy the following tcl code into it:

proc bad {{value ""}} {
    # do something
}

proc good {value} {
    # do something
}


This is valid tcl code, creating two functions (or procedures, als Tcl
also calls them). The first function has an optional argument. If that
argument isn't given, the argument variable 'value' will have the
default value ("" in the bad example) given in the proc definition. The
second is an example for a function. that always expects one argument.

- Put the point at the beginning of the proc bad and call
  tcl-end-of-defun (which is an alias of end-of-defun, therefor all
  keybindings to that will work). Now the point is (marked as: _P_
 
proc bad {{value ""}} _P_{
    # do something
}

This is wrong, this is not the end of the proc. In general,
tcl-end-of-defun does work. Look at the proc good. Placing the point at
the beginning of proc good and calling tcl-end-of-defun, the point is
where expected:

proc good {value} {
    # do something
}_P_

This is only an example of the mis-functioning. It seems always to show
up, if a proc definition has the syntax for one ore more optional
arguments with the empty string (given as "" in the code) as default
value.

Since tcl-eval-defun is implemented with the help of end-of-defun, it
suffers from this, too. 



In GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.10)
 of 2015-04-11 on linux-qg7d
Windowing system distributor `The X.Org Foundation', version 11.0.11203000

bug#23565: Acknowledgement (24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable)

[Rolf Ade]

I missed to note, that the reported misbehavior is still there in
emacs 25.0.94.1.

bug#23565: (24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable)

[Noam Postavsky]

tag 23565 + confirmed
quit

It has to do with the tcl-omit-ws-regexp which is used to set the
buffer local value of defun-prompt-regexp (why isn't it called
tcl-defun-prompt-regexp?). The current setting, along with commentary
is:

;; Here's another stab.  I think this one actually works.
;; We have to be careful that the open-brace following this regexp
;; is indeed the one corresponding to the function's body so
;; that end-of-defun works correctly.  Tricky cases are:
;;    proc foo { {arg1 def} arg2 } {
;; as well as
;;    proc foo { \n {arg1 def} \n arg2 } {
;; The current setting handles the first case properly but not the second.
;; It also fails if `proc' is not in column-0 (e.g. it's in a namespace).
(defconst tcl-omit-ws-regexp "^[^]\" \t\n#}][^\n\"#]+[ \t]+")

If I remove the " from the second character set, then the examples
given in this bug report work, but probably some others might break (I
don't know tcl well enough to come up with any).

;; this works for given examples
(defconst tcl-omit-ws-regexp "^[^]\" \t\n#}][^\n#]+[ \t]+")

A regexp based approach is probably always doomed to have some bad
cases, so perhaps the real fix is to define a
beginning-of-defun-function for tcl-mode.

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Tom Tromey]

I've checked in a fix for the tcl-end-of-defun problem.
I think it should fix tcl-eval-defun as well, but I didn't test this.

Tom

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Rolf Ade]

I'm sorry for the delay in replying; I was on vacation and saw your mail
just this day, after comming back.

Thank you for taking care!

Tom Tromey <tom@tromey.com> writes:
> I've checked in a fix for the tcl-end-of-defun problem.
> I think it should fix tcl-eval-defun as well, but I didn't test this.

I've build current master and did a few tests. Yes, it seems, that
tcl-end-of-defun as well as tcl-eval-defun now work for me as expected,
even in the reported-as-broken cases.

I'll copy masters tcl-mode.el over to my currently used 26.1
installation (and bytecompile, of course) and give it a try in my daily
Tcl coding, for a few days, to see, if I stumble over any unexpected
side-effect of the changes.

I'll report in a week or at most two (it varies a bit how much Tcl
coding I have to do and this is driven by others).

rolf

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Rolf Ade]

Rolf Ade <rolf@pointsman.de> writes:

> Tom Tromey <tom@tromey.com> writes:
>> I've checked in a fix for the tcl-end-of-defun problem.
>> I think it should fix tcl-eval-defun as well, but I didn't test this.
>
> I've build current master and did a few tests. Yes, it seems, that
> tcl-end-of-defun as well as tcl-eval-defun now work for me as expected,
> even in the reported-as-broken cases.
>
> I'll copy masters tcl-mode.el over to my currently used 26.1
> installation (and bytecompile, of course) and give it a try in my daily
> Tcl coding, for a few days, to see, if I stumble over any unexpected
> side-effect of the changes.
>
> I'll report in a week or at most two (it varies a bit how much Tcl
> coding I have to do and this is driven by others).

Well, beside the improvement (tcl-end-of-defun and tcl-eval-defun does
work better), which I again confirm I see one more (unexpected and
unwanted) effect.

With emacs 26.1:

- emacs -Q

- M-x tcl-mode

- Insert

do {
foo            

Indent the second line ("foo") with <Tab> and you get

do {
    foo


Do the same with master and you get

do {
foo

the second line ("foo") does not get indented with <Tab>. 

Something is special with "proc". If there is a proc before such code as
above the indentation works even with master, e.g. with

proc do {script} {
    eval $script
}

do {
foo

the line "foo" will be indented by <Tab> with 26.1 and master.

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Tom Tromey]

>>>>> "Rolf" == Rolf Ade <rolf@pointsman.de> writes:

Rolf> Indent the second line ("foo") with <Tab> and you get
Rolf> do {
Rolf>     foo

Thanks for trying this.
I suppose instead of the fix for the "namespace" thing, tcl-mode should
maybe just looking at brace depth for indentation.  I will take a deeper
look.

Tom

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Rolf Ade]

Tom Tromey <tom@tromey.com> writes:
>>>>>> "Rolf" == Rolf Ade <rolf@pointsman.de> writes:
>
> Rolf> Indent the second line ("foo") with <Tab> and you get
> Rolf> do {
> Rolf>     foo
>
> Thanks for trying this.
> I suppose instead of the fix for the "namespace" thing, tcl-mode should
> maybe just looking at brace depth for indentation.  I will take a deeper
> look.

Despite #32035 this is still open with master of today (but not with
26.2). Completely legal and commen code at the beginning befor the first
word out of tcl-proc-list are not indented as expected. Examples:

do {
something
}

if {$argc != 0} {
puts stderr "usage: $argv0"
}

set options {
-foo "default"
-bar "bardefault"
}


Expected indentation would be of course something like:

do {
    something
}

if {$argc != 0} {
    puts stderr "usage: $argv0"
}

set options {
    -foo "default"
    -bar "bardefault"
}


After the first word out of tcl-proc-list in the buffer all that
examples above will be indented as expected, no misbehaviour any more.

I happily confirm again, that master hasn't the original problem of this
bug report. And I'd really love to see that fixed, after it has plagued
me for 20 years.

But I'm afraid that the cure (current behavior of master) is worse than
the problem (26.2).

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Rolf Ade]

Rolf Ade <rolf@pointsman.de> writes:
> Tom Tromey <tom@tromey.com> writes:
>>>>>>> "Rolf" == Rolf Ade <rolf@pointsman.de> writes:
>>
>> Rolf> Indent the second line ("foo") with <Tab> and you get
>> Rolf> do {
>> Rolf>     foo
>>
>> Thanks for trying this.
>> I suppose instead of the fix for the "namespace" thing, tcl-mode should
>> maybe just looking at brace depth for indentation.  I will take a deeper
>> look.
>
> Despite #32035 this is still open with master of today (but not with
> 26.2). Completely legal and commen code at the beginning befor the first
> word out of tcl-proc-list are not indented as expected. Examples:
>
> do {
> something
> }
>
> if {$argc != 0} {
> puts stderr "usage: $argv0"
> }
>
> set options {
> -foo "default"
> -bar "bardefault"
> }
>
>
> Expected indentation would be of course something like:
>
> do {
>     something
> }
>
> if {$argc != 0} {
>     puts stderr "usage: $argv0"
> }
>
> set options {
>     -foo "default"
>     -bar "bardefault"
> }
>
>
> After the first word out of tcl-proc-list in the buffer all that
> examples above will be indented as expected, no misbehaviour any more.
>
> I happily confirm again, that master hasn't the original problem of this
> bug report. And I'd really love to see that fixed, after it has plagued
> me for 20 years.
>
> But I'm afraid that the cure (current behavior of master) is worse than
> the problem (26.2).

(I'm getting nervous - emacs-dev already talks about cutting an emacs-27
branch ... I'd really love to see this long standing bug to be fixed.
Let me recapitulate.)

The original bug of this report was fixed by

commit cd5bb4bf3dbad8941d25823f398b595b8f0edbb9
Author: Tom Tromey <tom@tromey.com>
Date:   Sun Jun 24 11:18:19 2018 -0600

This fix introduced a new unwanted indentation behaviour. Any code at
the beginning of a buffer in tcl-mode before the first word that match
an element out of tcl-proc-list (or so it looks like, I haven't really
understand all details but I've provided some examples earlier in the
thread) will not be indented as typically expected.

I'm afraid the behaviour of current master is more worse than it was
before of cd5bb4bf3dbad8941d25823f398b595b8f0edbb9. E.g. work with a
notable fraction of the tcl files included in the tcl core distribution
(https://core.tcl-lang.org/tcl/download) would suffer from this.

So before release of emacs 27 this commit should be reverted. Or, much
better, because the commit fixes the original problem (which I really
love to see fixed), it should be improved.

I came up with the following. The simple idea is to keep the new
behaviour where it shines and fall back to the old way otherwise.

Commit message:

Fix tcl-mode indentation after fix of bug#23565

* lisp/progmodes/tcl.el (tcl-calculate-indent) Fall back to old
  indentation method for tcl code at the start of the buffer before the
  first word matching an element out of tcl-proc-list.

Copyright-paperwork-exempt: yes

diff --git a/lisp/progmodes/tcl.el b/lisp/progmodes/tcl.el
index 0fd3d6d1bf..db97230480 100644
--- a/lisp/progmodes/tcl.el
+++ b/lisp/progmodes/tcl.el
@@ -817,7 +817,9 @@ tcl-calculate-indent
           found-next-line)
       (if parse-start
          (goto-char parse-start)
-       (beginning-of-defun))
+       (if (not (beginning-of-defun))
+            (let ((beginning-of-defun-function nil))
+              (beginning-of-defun))))
       (while (< (point) indent-point)
        (setq parse-start (point))
        (setq state (parse-partial-sexp (point) indent-point 0))

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Lars Ingebrigtsen]

Rolf Ade <rolf@pointsman.de> writes:

> * lisp/progmodes/tcl.el (tcl-calculate-indent) Fall back to old
>   indentation method for tcl code at the start of the buffer before the
>   first word matching an element out of tcl-proc-list.
>
> Copyright-paperwork-exempt: yes
>
> diff --git a/lisp/progmodes/tcl.el b/lisp/progmodes/tcl.el
> index 0fd3d6d1bf..db97230480 100644
> --- a/lisp/progmodes/tcl.el
> +++ b/lisp/progmodes/tcl.el
> @@ -817,7 +817,9 @@ tcl-calculate-indent
>            found-next-line)
>        (if parse-start
>           (goto-char parse-start)
> -       (beginning-of-defun))
> +       (if (not (beginning-of-defun))
> +            (let ((beginning-of-defun-function nil))
> +              (beginning-of-defun))))

Hm...  This does fix the indentation issue, but I wonder whether
tcl-beginning-of-defun-function should be fixed instead so that the
movement commands work better?

I don't use TCL, but the problem is that top-level (outside of
functions) that tcl-beginning-of-defun-function doesn't go to the start
of forms like:

if {$argc != 0} {
    puts stderr "usage: $argv0"
}

But what should beginning-of-defun do outside of functions?  It's not
documented in the doc string of that function...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#23565: 24.5; tcl-end-of-defun and tcl-eval-defun doesn't work reliable

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Hm...  This does fix the indentation issue, but I wonder whether
> tcl-beginning-of-defun-function should be fixed instead so that the
> movement commands work better?

There were no comments in two weeks, and the patch does fix the problem
at hand, so I've now applied it to Emacs 27.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no