Improve handling of carriage return ("\r")

[plexus]

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 #1677 (which was closed but not actually implemented, only worked around)

TODOS

• Update existing bencode test since the parsing of "\r" has changed

---

Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

• The commits are consistent with our contribution guidelines
• You've added tests (if possible) to cover your change(s)
• All tests are passing (make test)
• All code passes the linter (make lint) which is based on elisp-lint and includes
  • byte-compilation, checkdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.
• You've updated the changelog (if adding/changing user-visible functionality)
• You've updated the user manual (if adding/changing user-visible functionality)

Uh are these instructions up to date? I'm getting:

➜ make test
make: Nothing to be done for 'test'.

➜ make lint
make: *** No rule to make target 'lint'.  Stop.

[bbatsov]

Feedback welcome as I'm not sure of the performance impact of dealing with each
character individually.

I have a feeling the impact will be quite significant on big chunks of output, but without runnings some benchmarks that's just a guess.

Also not sure what the effect is of propertizing and
running cider-repl-preoutput-hook per character.

I guess it's going to break it, as currently the hook functions (like ansi-color-apply expect the complete output string). On the other hand that might be somewhat broken even now for streamed output, as we can't really know where the chunk boundaries are going to fall - in some cases the filter functions might produce weird results for the individual chunks.

One idea that comes to my mind is to look for \r in the output before trying to do something special. This will probably cover most scenarios. I'm also wondering how is this handled in a typical shell, as this might give us some inspiration.

[bbatsov]

Uh are these instructions up to date? I'm getting:

Ops, sorry about that! We just switch to eldev and removed the Makefile. (it's now eldev test and eldev lint)

[plexus]

Going to close this as this is a fairly minor issue for me that will require a fairly major investment, which I'd currently rather put somewhere else. If someone is interested in tackling this however I'm happy to pair/mentor/provide an extra pair of eyes.

[johngit22]

Thanks for sharing 😊

…

On Fri, Aug 21, 2020, 6:28 AM Arne Brasseur ***@***.***> wrote: Closed #2878 <#2878>. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2878 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYXHWNJCZKRB47MPUJO243SBZD4DANCNFSM4PF2S2VQ> .