Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REPL doesn't interpret carriage return #1677

Closed
thomas-morgan opened this issue Apr 14, 2016 · 5 comments
Closed

REPL doesn't interpret carriage return #1677

thomas-morgan opened this issue Apr 14, 2016 · 5 comments
Labels

Comments

@thomas-morgan
Copy link

I entered (println "foo\rbar") in the Cider REPL, hoping to see

bar
nil

Instead I saw

foobar
nil

This breaks progress bars among other things, as reported here: http://stackoverflow.com/questions/34649151/progress-indicator-bar-auto-changed-line-in-cider-nrepl-buffer/34651898#34651898.

For comparison, (display "foo\rbar\n") displays bar in the REPL started by run-guile.

Environment & Version information

CIDER version information

;; CIDER 0.11.0 (Bulgaria), nREPL 0.2.12
;; Clojure 1.8.0, Java 1.8.0_60

Lein/Boot version

Leiningen 2.6.1 on Java 1.8.0_60 OpenJDK 64-Bit Server VM

Emacs version

GNU Emacs 24.5.2 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.28) of 2015-09-25 on machine

Operating system

NixOS 15.09

@bbatsov bbatsov added the bug label Apr 30, 2016
@bbatsov
Copy link
Member

bbatsov commented Apr 30, 2016

Seems you're right - this is a bug. I'm not sure what's causing it, though. The message we send to the nREPL server seems correct to me:

(--> 
  ns  "cider-demo.core"
  op  "eval"
  session  "4bab8d15-cbc4-4ec7-a6ad-087c51c37845"
  code  "(println \"foo\\rbar\")\n"
  file  "*cider-repl cider-demo*"
  line  58
  column  17
  id  "55"
)

@bbatsov
Copy link
Member

bbatsov commented Jul 24, 2016

@dpsutton Maybe you'd like to investigate another REPL bug? :-)

@dpsutton
Copy link
Contributor

This appears to be a bug in the nrepl--bdecode-1 function,


;; normalise any platform-specific newlines
        (let* ((original (buffer-substring-no-properties beg end))
               (result (replace-regexp-in-string "\r" "" original)))
          (cons nil (nrepl--push result stack))))))

in nrepl-client.el. I'm gonna go learn about platform specific newlines as I've never really understood the difference between \r and \r\n, etc. If i remove that line, i'm getting ^M in the buffer, which is the perennial problem with git on multiple platforms. So I just need to learn the actual issue at hand instead of a hand-wavy idea about newlines.

@dpsutton
Copy link
Contributor

dpsutton commented Jul 25, 2016

I guess I don't know if a \r by itself is valid. I've always seen the windows style as \r\n, which is why the code was only removing the \r by itself.

;; ======================================================================
user> (print "bob\r\nsmith\n")
bob
smith
nil
user> (print "bob\rsmith\n")
bob
smith

The following works, but I'm not sure if its hacky, nor if its correct. I'll play with this today and see what I can get

;; normalise any platform-specific newlines
        (let* ((original (buffer-substring-no-properties beg end))
               (result (replace-regexp-in-string "\r\n" "\n" original))
               (result (replace-regexp-in-string "\r" "\n" result)))
          (cons nil (nrepl--push result stack))))))

@bbatsov
Copy link
Member

bbatsov commented Oct 10, 2016

Guess there's not much more we can do about this now, so I'll close this. Thanks for fixing the handling for \r @dpsutton.

@bbatsov bbatsov closed this as completed Oct 10, 2016
plexus added a commit to plexus/cider that referenced this issue Jul 23, 2020
A CARRIAGE RETURN character moves the head of the teletype printer (the
carriage) to the beginning of the line, without scrolling the paper (this is
done by the LINE FEED "\n")

In a contemporary terminal emulator this translates to moving the cursor to the
beginning of the current line. Any subsequent output then overwrites what was
already on that line. This is how terminal progress bars are implemented.

The existing implementation incorrectly treated "\r" as "\n", thus inserting a
newline, instead of moving to the beginning of the current line.

Instead iterate over the output and deal with "\r" and "\n" the way terminals
do, "\r" (ASCII 13) moves to the beginning of the line, "\n" (ASCII 10) moves to
the beginning of the next line. Any output overwrites output on the same line
after the cursor position.

Sample code:

```
(println "aaaa\rbb\ncccc\rdd")
;; Outputs:
;; bbaa
;; ddcc

(dotimes [i 20]
  (print "\r[" (inc i) "]")
  (flush)
  (Thread/sleep 500))
;; prints a counter in place
```

Feedback welcome as I'm not sure of the performance impact of dealing with each
character individually. Also not sure what the effect is of propertizing and
running cider-repl-preoutput-hook per character.

Fixes clojure-emacs#1677 (which was closed but not actually implemented, only worked around)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants