nonsequitur / inf-ruby

219 stars 68 forks source link

Multiple irb prompt strings in send-region output #49

Closed sankalp-khare closed 1 year ago

sankalp-khare commented 10 years ago

On executing ruby-send-region, the output contains a large number of completely unnecessary irb prompt strings.

image

Check the image for an example.

Kindly advise how to resolve this. Thanks!

Using

dgutov commented 10 years ago

This is the expected behavior. The REPL receives input line-by-line, and displays intermediate prompts for each of them.

I'm not sure fixing that is worth the effort, but patches are welcome.

stardiviner commented 9 years ago

Maybe inf-ruby can remove this prompt from the retrive.

dgutov commented 9 years ago

I suppose we could use comint-preoutput-filter-functions for that. But it's seems non-trivial to only filter out the intermediate prompts, but keep the final ones.

comint-send-string works asynchronously, so it's hard to limit filtering just to ruby-send-region output.

dgutov commented 9 years ago

Note: comint-output-filter already tries to filter out repeated prompts, but fails in our case because the IRB prompt contains a command counter.

sideshowcoder commented 7 years ago

As a tip it seems to work fine for pry not perfect but good enough for sure, this is my .pryrc

Pry.config.pager = false
Pry.config.correct_indent = false if ENV['INSIDE_EMACS']
Pry.color = true

sample output

[1] pry(main)> 
eval <<'--inf-ruby-124f1e97b51b240f-22656-50635-359892--', (defined?(IRB) && IRB.conf[:MAIN_CONTEXT] && IRB.conf[:MAIN_CONTEXT].workspace.binding) || (defined?(Pry) && Pry.toplevel_binding), "/a/path/somewhere", 28
[1] pry(main)*         class A
          def initialize
            @a = 1
          end
        end

        A.new
[1] pry(main)* 
--inf-ruby-124f1e97b51b240f-22656-50635-359892--
[1] pry(main)* => #<A:0x007fcf3a5ba2f0 @a=1>
dgutov commented 7 years ago

@sideshowcoder How does this look in IRB?

sideshowcoder commented 7 years ago

In IRB I'm seeing what @sankalp-khare shows in the screenshot above, but I've just set Pry as my default and this works for me as shown above.

dgutov commented 7 years ago

Cool, thanks.

bo-tato commented 1 year ago

I get the same as @sankalp-khare screenshot, with irb or pry, including when I set those pry config settings @sideshowcoder mentions. In pry the intermediate prompts end with * and the final ones with > so I just did a preoutput-filter:

(defun my/filter-ruby-repl (string)
  "filter out pry's prompts that end in '* ', so the ruby-send-* commands
don't show extra prompts"
  (if (and (string-match inf-ruby-prompt-pattern string)
           (s-suffix? "* " string))
      ""
    string))

(defun my/add-ruby-comint-filter ()
  (add-hook 'comint-preoutput-filter-functions #'my/filter-ruby-repl))

(add-hook 'inf-ruby-mode-hook #'my/add-ruby-comint-filter)

it's working for me but seems like a horrible hack, I don't know what would be the proper solution. @dgutov the comint-output-filter related to filtering repeated prompts sounds promising but I can't find it, can you link the source of that?

bo-tato commented 1 year ago

There is another duplicate of this issue here https://github.com/nonsequitur/inf-ruby/issues/166 For reference I switched from that preoutput filter which doesn't work 100% to just adding to my pryrc:

Pry.config.prompt.prompt_procs[1] = proc { "" } if ENV["INSIDE_EMACS"]

It just sets it so it doesn't display further prompts for multline input when running inside emacs.

irb has the command line option --inf-ruby-mode which makes in not display the intermediate prompts and fixes this issue. I wonder why irb has --inf-ruby-mode option but it seems inf-ruby-mode makes no use of it? It would make since for inf-ruby-mode to pass that option to irb by default? The only problem with it to address is that ruby-print-result expects the output to be prefixed with " => ", which is no longer the case with irb in --inf-ruby-mode, so ruby-print-result doesn't find the output and doesn't show it in a pretty overlay anymore.

bo-tato commented 1 year ago

btw if anyone knows comint mode and how this is addressed with other languages please help me out, I looked into it a while and am confused, for example if we do:

$ echo -ne "for i in range(2):\n    print(i)" | python3 -i
Python 3.11.4 (main, Jun  7 2023, 10:13:09) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> ... ... 
0
1
>>> 

we see the same problem ruby mode is having, python repl displays it's intermediate prompt "..." repeatedly on the same line, but when we run it inside emacs, with comint-send-string, python repl does not display the intermediate prompt! And it doesn't seem to be some comint filtering going on, I strace python process to be sure and it is not displaying "..." when input is sent from emacs. But when we type manually in python repl buffer inside emacs then it does nicely show the prompt as we want:

>>> for i in [1,2]:
...     print(i)
... 
1
2
>>> 

I have no idea how it is knowing to not show the extra intermediate prompts only when input comes from the python-send... commands

dgutov commented 1 year ago

First of all, this bug report is about repeating prompts when one calls ruby-send-region or a similar command from another buffer. This shouldn't happen during regular usage (when someone types in the REPL buffer).

If that happens during normal usage, it would be better to continue in a new issue with more information provided. Report #166 is broken in the same way: the first message is about send-region only, and the second one shows some outright weird behavior in a video. So if you have a problem of the second sort, please open a new report with some details (versions of everything), and we can continue there.

bo-tato commented 1 year ago

Ah sorry, I hadn't watched the video, just the screenshot at top of issue that looked like the same thing. Yea I haven't had any issues typing in the REPL like that video, just with showing the repeated prompts when calling the ruby-send-* commands from a ruby buffer

dgutov commented 1 year ago

About your questions:

@dgutov the comint-output-filter related to filtering repeated prompts sounds promising but I can't find it, can you link the source of that?

Apparently this was about the code which got removed in https://github.com/emacs-mirror/emacs/commit/c0936672876bccc. The message says it didn't work anyway, but maybe it can serve as a starting point for further study.

I wonder why irb has --inf-ruby-mode option but it seems inf-ruby-mode makes no use of it?

I remember having some problems with the lack of continuation prompts, though I can't currently recall which ones.

But note that it's only an option for Irb. Even if we tried using it again, what about Pry?

dgutov commented 1 year ago

Speaking of Python, there is python-shell-output-filter, which at the end says this:

    (when (string-match
           python-shell--prompt-calculated-output-regexp
           python-shell-output-filter-buffer)
      ;; Some shells, like IPython might append a prompt before the
      ;; output, clean that.
      (setq python-shell-output-filter-buffer
            (substring python-shell-output-filter-buffer (match-end 0)))))

Though apparently it's not used with the regular Python shell.

bo-tato commented 1 year ago

ah that makes sense it used to be there but got removed. I'd read on a stackoverflow response also that comint had code to filter duplicated prompts and was thoroughly confused when I read comint and can't find reference to that. I figured why python doesn't have the problem with showing repeated intermediate prompts, it's because the python-send-* commands are using eval so the newlines are escaped inside a python string rather then the process actually seeing newlines, from comint trace output:

1 -> (comint-send-string #<process Python> "__PYTHON_EL_eval(\"if True:\\n    print( 1)\", \"<string>\")\n")

ie it is sending one line: __PYTHON_EL_eval(\"if True:\\n print( 1)\", \"<string>\")\n")

dgutov commented 1 year ago

Hmm, can we do that too?

Note issue #172, though: long spans without actual newlines can lead to problems.

bo-tato commented 1 year ago

interesting... it seems python mode also has a problem with long lines, I send a 4K+ line of 1 + 1 + 1 ..., pasted in the repl it works fine, but with python-send-statement it errors:

Traceback (most recent call last):
  File "<string>", line 19, in __PYTHON_EL_eval
RecursionError: maximum recursion depth exceeded while traversing 'expr' node

but it seems that is some other error based on something else __PYTHON_EL_eval is doing. Can we just send one line eval "(region to eval as string with \n instead of newline)"?

I remember having some problems with the lack of continuation prompts, though I can't currently recall which ones.

Yea I guess it makes sense not to use by default as it makes the experience typing multiline statements into the repl worse. Personally I'd never do that, I just use ruby-send-* commands and use a scratch buffer if needed rather than type in REPL, but it should be a good experience also for the people that like to type in REPL.

Edit: by the way this is the stackoverflow answer in question that says comint performs duplicate prompt elimination. If someone wants to comment with that commit dgutov linked explaining it got removed so future readers aren't confused like me. I don't have enough reputation on stackoverflow to comment lol

bo-tato commented 1 year ago

Another possibility would be for the ruby-send commands to send:

__orig_prompt_mode=IRB.conf[:PROMPT_MODE];IRB.conf[:PROMPT_MODE] = :INF_RUBY; (actual region to send); IRB.conf[:PROMPT_MODE] = __orig_prompt_mode

and could do the same for pry, saving and disabling the intermediate prompt only when sending from inf-ruby, I don't know whether that's more hacky and worse that sending as single string with eval.

dgutov commented 1 year ago

by the way this is the stackoverflow answer in question that says comint performs duplicate prompt elimination. If someone wants to comment with that commit dgutov linked explaining it got removed so future readers aren't confused like me. I don't have enough reputation on stackoverflow to comment lol

I've done that now. Probably should make an edit to the answer directly sometime later.

dgutov commented 1 year ago

Another possibility would be for the ruby-send commands to send <..> and could do the same for pry, saving and disabling the intermediate prompt only when sending from inf-ruby, I don't know whether that's more hacky and worse that sending as single string with eval.

I don't know. :slightly_smiling_face: You can try implementing and running with that approach for a bit, and report back. I suppose the main downside, aside from not being tried-over-time yet, is that every different REPL might require a different piece of code to remove-and-restore the prompt. And some (future ones?) might not have such capability, so that can be a friction point.

So personally I recommend adapting the current solution first.

Can we just send one line eval "(region to eval as string with \n instead of newline)"?

Should be doable, yes: instead of multiple comint-send-* calls, construct a longer string in advance and then send it.

Note that simply doing that will make #172 much more likely to happen: IIUC escaped newlines do not count as newlines for the purposes of that bug. So ideally our code would iterate through the string, escape newlines and etc, and count characters: as soon as it encounters a span of chars 4096 long, it will have to go back and un-escape the last newline, and start counting again. And if it doesn't manage to split the strings into pieces <4096 long this way, signal a user error (that would be the best fix we can do for #172 too).

That does sound like a bit of a pain to implement, but hopefully it's manageable.

bo-tato commented 1 year ago

wow I'm learning more about messy internals of pty and emacs than I planned on lol. I checked how other languages handle this, the lisp/scheme/clojure that connect to repl over socket rather than stdin/stdout is the right way I think, but for other languages that talk to repl over stdin/stdout, tuareg mode for ocaml is spawning repl with -nopromptcont by default which makes it not show intermediate prompt on multiline input, and inf-elixir mode has this same problem actually, of showing duplicate intermediate prompts when you send-region.

I went to lookup documentation for eval and saw you can pass it file name and line number and thought that's cool we can have it show error messages with the right line numbers and stuff for code sent to repl with ruby-send-* when I check and... inf-ruby is already doing that! I didn't realize it's already using eval, just then sending the code in a heredoc over multiple lines rather than passing it as a single string argument.

I can think of maybe 3 ideas, what do you think is best?

  1. split string into pieces <4096 like you say, I think it should always be possible? well not if you only use the newlines already in the string, but if you send as something like:
    'start of long escaped line'\
    'some more on the same line'\
    'etc'
  2. Just write the region to a temporary file, and send eval(File.read(filename),...) it's simpler should always work no problems, but it's slower as it's writing a file on disk for every snippet we eval.
  3. figure how to work around https://github.com/nonsequitur/inf-ruby/issues/172 I just did some testing and wrote more there, basically it seems long lines work fine when process-connection-type is nil (no pty), but there is a problem of it echoing the input, which could be worked around with using comint-process-echoes set to t and comint-send-input instead of comint-send-string or comint-send-region.
  4. remove-and-restore the intermediate prompt, but like you say that will require custom code for every different repl, where plain ruby 'eval' should always work.

I'm thinking 1 is the least problematic?

dgutov commented 1 year ago

wow I'm learning more about messy internals of pty and emacs than I planned on lol

Story of my life ;-)

3 sounds interesting, but one would have to implement it first and try running with different shells. I'm not 100% sure why comint always uses the connection type pty, but there might as well be practical reasons for it. After all, running as a pipe is often a hint for the inferior process that it's not expected to be interactive.

2 is interesting as well, I would use it as a backup plan.

Good note on 2: indeed, an eval expression can be split using string quotes. We'd just need to escape the contents first (and escape whatever escapings are already there), and then split the result as necessary. That should solve both of the issues, actually. Since it's the closest to how we do things now, I'd prefer to try this fix first.

bo-tato commented 1 year ago

yea I think behavior of pry and irb could be slightly different when they think they're not being run interactive. So far I noticed the help message in pry doesn't have a newline at the end and puts the prompt on the same line, there's probably other quirks.

I checked what python-mode is doing and the relevant code from python-shell-send-string is:

(if (or (null (process-tty-name process))
            (<= (string-bytes code)
                (or (bound-and-true-p comint-max-line-length)
                    1024))) ;; For Emacs < 28
        (comint-send-string process code)
      (let* ((temp-file-name (with-current-buffer (process-buffer process)
                               (python-shell--save-temp-file string)))
             (file-name (or (buffer-file-name) temp-file-name)))
        (python-shell-send-file file-name process temp-file-name t)))

If there's no pty, it will send as a long string, otherwise it checks that the length is under comint-max-line-length which is 4096 for me, defaulting to 1024 if comint-max-line-length doesn't exist, from the comment I assume it was introduced in emacs 28. Then if the string is too long for pty/comint to handle, rather than implementing some messy string splitting logic, it's backup strategy is simply writing it to a temp file and eval the temp file.

dgutov commented 1 year ago

Then if the string is too long for pty/comint to handle, rather than implementing some messy string splitting logic, it's backup strategy is simply writing it to a temp file and eval the temp file.

Sounds good, and should be easier to implement than the splitting thing. This code is there since 2021, so it has seen some field testing too. :+1:

dgutov commented 1 year ago

Should be fixed with #175.