Shell-mode: File completion breaks editing previous commands

Bob Rogers <rogers-emacs@rgrjr.dyndns.org> writes:

[...]

Is this still a problem?

--

(domestic pets only, the antidote for overdose, milk.)

bloggy blog http://lars.ingebrigtsen.no/

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

More information, but was apparently not given, so I'm closing this bug

report. If this is still a problem, please reopen the bug report.

--

(domestic pets only, the antidote for overdose, milk.)

bloggy blog http://lars.ingebrigtsen.no/

close 114

Lars Magne Ingebrigtsen writes:

> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> >> I have seen this intermittently for a long time now, but still haven't

> >> been able to reproduce it reliably.

> >

> > Is this still a problem?

>

> More information, but was apparently not given, so I'm closing this bug

> report. If this is still a problem, please reopen the bug report.

This is still broken in Emacs 23 at least. I'm happy to provide any extra

info you need,

Richard

GNU Emacs 23.2.1 (x86_64-slackware-linux-gnu, GTK+ Version 2.18.9) of 2010-05-08 on midas64

richard@rswheeldon.com writes:

Ok; I'll reopen the report.

--

(domestic pets only, the antidote for overdose, milk.)

bloggy blog http://lars.ingebrigtsen.no/

For anyone like me who had trouble following the recipe:

emacs -Q -f shell

touch foo bar RET

echo foo RET

C-u 2 C-p

C-u 5 C-f

; point now on "f" of "echo foo"

baTAB

; completes so that line reads "echo bar foo", with cursor on "f".

RET

; -> tries to run the command "foo"

If you type "bar " by hand rather than using tab completion, there is no

problem.

Ah, I guess there's some funny `field' text property going on.

Stefan

Stefan Monnier wrote:

I think you guess correctly. Looks like the completion does not put an

`input' property on the text that it inserts.

tag 114 - moreinfo

[ Hi, Miles, I think you're the "fields in comint" guru; coud you give

us a hand? ]

Indeed, the problem is that previous commands are given a `field' text

property of value `input', but since this is a text property, insertion

of text within this line won't have that property unless it's done via

insert-and-inherit.

This can be seen as above with completion, but it also happens with C-y.

Maybe the best fix is to get rid of this `input' field value (the rest

of the text already gets an `output' field value, so a nil value would

still work as a field).

E.g. the patch below seemed to fix the problem, but I'm not sure what

other consequences it might have.

Stefan

=== modified file 'lisp/comint.el'

--- lisp/comint.el 2011-10-03 16:49:56 +0000

+++ lisp/comint.el 2011-10-04 02:05:59 +0000

@@ -849,8 +849,7 @@

(and (< pos (field-end pos))

(setq field (field-at-pos pos))

(setq input (field-string-no-properties pos))))

- (if (or (null comint-accum-marker)

- (not (eq field 'input)))

+ (if (or (null comint-accum-marker) field)

;; Fall back to the global definition if (i) the selected

;; buffer is not a comint buffer (which can happen if a

;; non-comint window was selected and we clicked in a comint

@@ -1803,8 +1802,7 @@

(add-text-properties

beg end

'(mouse-face highlight

- help-echo "mouse-2: insert after prompt as new input"

- field input))))

+ help-echo "mouse-2: insert after prompt as new input"))))

(unless (or no-newline comint-use-prompt-regexp)

;; Cover the terminating newline

(add-text-properties end (1+ end)

@@ -2153,7 +2151,7 @@

the current line with any initial string matching the regexp

`comint-prompt-regexp' removed."

(let ((bof (field-beginning)))

- (if (eq (get-char-property bof 'field) 'input)

+ (if (null (get-char-property bof 'field)) ;Not `output'.

(field-string-no-properties bof)

(comint-bol)

(buffer-substring-no-properties (point) (line-end-position)))))

@@ -2473,7 +2471,7 @@

(while (/= n 0)

(unless (re-search-backward regexp nil t dir)

(error "Not found"))

- (when (eq (get-char-property (point) 'field) 'input)

+ (unless (get-char-property (point) 'field)

(setq n (- n dir))))

(field-beginning))))

(goto-char pos))))

@@ -2520,7 +2518,7 @@

(setq input-pos (point-max)))

;; stop iterating

(setq n 0))

- ((eq (get-char-property pos 'field) 'input)

+ ((null (get-char-property pos 'field))

(setq n (if (< n 0) (1+ n) (1- n)))

(setq input-pos pos))))

(when input-pos

Stefan Monnier wrote:

For one, it leaves the help echo ("mouse-2: insert after prompt as new

input") and mouse highlight properties broken in two on either side of

the portion inserted by completion.

But that's also the case before my patch, right? So it's not

a consequence of my patch.

Stefan

Stefan Monnier wrote:

OK then, pretend I said:

I'm also not sure what consequences simply removing this field property

will have.

However, I noticed that your patch leaves the help-echo and mouse-face

properties missing on the completed text, which is a little unaesthetic.

An alternative solution that avoids this issue would be if callers of

completion could optionally request that it insert-and-inherit the

completed text rather than just insert it. I could imagine that other

callers of completion might need such a feature. But I have no idea if

this is feasible.

I think there are two issues:

1- handling the `insert' case as well as possible, because a lot of code

uses just `insert' (tho in practice very few cases seem to show up:

yank, completion, any other?).

2- changing (some) users of `insert' such as completion to use

insert-and-inherit.

(2) might be good, indeed, tho I'm not completely sure what are the consequences.

Stefan

I've installed the patch that makes comint use nil rather than `input'

for the field property.

And I've also installed a patch that makes completion use

insert-and-inherit for good measure.

Stefan