Echoed input not responding to comint-process-echoes or ess-eval-visibly

[BrendanHalpin]

Commands are duplicated in the output. Setting comint-process-echoes to t, or changing ess-eval-visibly seems to have no effect.

In an XFCE-terminal, Stata doesn't duplicate output. In an Emacs shell, it does, but it responds to comint-process-echoes. In ESS it duplicates but does not respond to comint-process-echoes.

[vspinu]

. In ESS it duplicates but does not respond to comint-process-echoes.

This could be because the output is not exactly the same as input.

[BrendanHalpin]

I don't think so, because if I start Stata in an Emacs shell, it duplicates output until I set comint-process-echoes to t, and then it behaves properly.

I am currently working on the hypothesis that it has to with the prompt regexps, which seem to be the only things that differ across standard comint (e.g., Stata in a shell), Stata in inferior-ess and R in inferior-ess. I need to do some more exploration.

[BrendanHalpin]

I've looked at this a bit more carefully. I have found that if comint-process-echoes is t, comint-send-input does delete one superfluous copy of the input, but only after inferior-ess-input-sender inserts a second copy.

I have "fixed" this by editing inferior-ess-input-sender, changing the invisibly parameter of ess-eval-linewise to t. Since this is only called if comint-process-echoes is t, it seems logical. Stata behaves correctly, and the change doesn't seem to have any effect on R or Julia.

However, I hesitate to suggest this as a fix as I don't know the codebase very well: am I missing some negative consequences of this change?

(defun inferior-ess-input-sender (proc string)
  (inferior-ess--interrupt-subjob-maybe proc)
  (let ((comint-input-filter-functions nil)) ; comint runs them, don't run twice.
    (if comint-process-echoes
        (ess-eval-linewise string t nil ess-eval-empty) ;; change
      (ess-send-string proc string))))

[lionel-]

However, I hesitate to suggest this as a fix as I don't know the codebase very well: am I missing some negative consequences of this change?

No worries, I still feel lost in the ESS internals after many years of working on various parts of it. I don't understand the relationship between eval visibility and process echoes. I think we shouldn't make any change until we have a global view of what is going on though, which is difficult in part because of all the moving parts, and in part because ESS is generic across REPLs and modes.

I'm surprised to see that for R, the value of comint-process-echoes depends on the value of visibility: https://github.com/emacs-ess/ESS/blob/719a6501/lisp/ess-r-mode.el#L2333. Not sure why. My immediate thought is that changing visibility requires restarting the processes to avoid inconsistent behaviour, which is a problem.

These issues are probably related to ess-eval-linewise inserting inputs in the process buffer: https://github.com/emacs-ess/ESS/blob/96ba4876/lisp/ess-inf.el#L1525-L1528. It seems that ess-eval-linewise inserts the input in the buffer, STATA echoes another copy, and then comint only deletes a single one?

Since changes to the input sender will affect R, we will need unit tests covering the various cases.

[BrendanHalpin]

"It seems that ess-eval-linewise inserts the input in the buffer, STATA echoes another copy, and then comint only deletes a single one?"

Exactly.

Grepping the lisp sub-directory, it seems that only R-mode and S-mode explicitly do anything to comint-process-echoes.
For now I'll re-iterate that it seems logical that ess-eval-linewise should be set to invisible when called conditionally on comint-process-echoes being true, but since testing beats logic I'll continue to experiment.

[lionel-]

Continuing the discussion from https://github.com/emacs-ess/ess-stata-mode/issues/10#issuecomment-818610904.

If comint-process-echoes is set to nil, then inferior-ess-input-sender shouldn't insert a superfluous copy? Am I missing something?

[BrendanHalpin]

Is it not the opposite? If the inferior process doesn't echo the input, ESS needs to insert it. If it does (and ESS knows via comint-process-echoes being set to t), there is no need to insert it, and ess-eval-linewise should have invisibly set to t (when called by inferior-ess-input-sender.

[lionel-]

I don't think so, there is no need to insert anything in the general case because the user input is already in the process buffer. inferior-ess-input-sender is only called on input typed in the process buffer, not on input sent from another buffer. This is why the input is already sitting there when the sender is called.

I don't understand:

• Why there is a different code path when visible eval is turned on.
• Why setting comint-process-echoes to nil (which should disable that code path) doesn't make a difference.

I may be completely wrong about how any of this works.

[BrendanHalpin]

I'm confused by the variety of ways ESS evaluates code (in the inferior-process buffer, linewise from the source file, block evaluation, ess-command in the background, etc).

However, do we understand the same of comint-process-echoes? I read it as saying that (if non-nil) the inferior process echoes the input so comint needs to delete one copy. This doesn't happen currently (in the process buffer, with comint-process-echoes set to t, the invisibly parameter set to nil means a third copy of the input is inserted, and only one deleted).

[lionel-]

However, do we understand the same of comint-process-echoes? I read it as saying that (if non-nil) the inferior process echoes the input so comint needs to delete one copy. This doesn't happen currently (in the process buffer, with comint-process-echoes set to t, the invisibly parameter set to nil means a third copy of the input is inserted, and only one deleted).

That's my understanding as well. I think comint-process-echoes should be nil and ESS shouldn't insert a copy from the sender. From my understanding of the code, it currently only does so when comint-process-echoes is set to t by calling ess-eval-linewise which inserts each line of code in the process buffer.

I think you may be right that we should turn on the invisible parameter in the ess-eval-linewise call in the sender. It would also be nice to simplify the sender, if possible, by removing the conditional path altogether, since no one seems to understand why it's there.

[BrendanHalpin]

OK, a trial with

(defun inferior-ess-input-sender (proc string)
  (inferior-ess--interrupt-subjob-maybe proc)
  (let ((comint-input-filter-functions nil)) ; comint runs them, don't run twice.
    (ess-eval-linewise string comint-process-echoes nil ess-eval-empty)))

seems to behave well in R and Stata buffers (with comint-process-echoes set to t or nowait in Stata).

[lionel-]

Encouraging.

(with comint-process-echoes set to t or nowait in Stata)

Did you mean ess-eval-visibly?

If not can you also do some experiments to see how this interacts with ess-eval-visibly? Beware you might need to restart the process after changing ess-eval-visibly because of the conditional initialisation here: https://github.com/emacs-ess/ESS/blob/ee2a2808/lisp/ess-r-mode.el#L2384. (Maybe this conditional init should be removed as well with this simplified sender.)

[BrendanHalpin]

I see comint-process-echoes as a way to tell ESS something about the underlying program (and so it should have different values for different programs), and ess-eval-visibly as an instruction to ESS about how to behave (so that you could possibly have different values for the same program for different purposes). I may well be wrong about the latter part, but it is why I used comint-process-echoes.

I will experiment with e-e-v (later, busy now).

[BrendanHalpin]

Adding (setq-local ess-eval-visibly nil) to the definition of ess-stata-mode fixes ess-eval-region but doesn't change inferior-ess-send-input (RET in the inferior process buffer), ess-eval-region-or-line-visibly-and-step (C-RET in the source-file buffer) or ess-command.