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

Handle lone carriage returns as newline #1814

Merged
merged 1 commit into from
Jul 27, 2016

Conversation

dpsutton
Copy link
Contributor

  • The commits are consistent with our [contribution guidelines][1]
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (make test)
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)
  • You've updated the refcard (if you made changes to the commands listed there)

Substitute newline for single carriage returns

#1677

this doesn't totally solve the issue actually. The issue poster was hoping that "foo\rbar" would right foo, return to the beginning of the same line, and then write bar. However, we were stripping carriage returns with no action. We now act as lein repl does, replacing this lone carriage returns with newlines.

The fundamental issue was that text-based status bars didn't work, and it sounds like we don't have plans to fix this, or at least not at the moment. There are lots of things to consider like markers, how we parse up the bencoded string to know when to move to next line, when to zap over current line, etc, and that would be a much larger change.

@bbatsov
Copy link
Member

bbatsov commented Jul 27, 2016

The changes look good. Mention them in the changelog and we're good to go.

The fundamental issue was that text-based status bars didn't work, and it sounds like we don't have plans to fix this, or at least not at the moment.

Well, I wouldn't mind fixing this, but as you said - it's probably going to require a bit of work.

@dpsutton
Copy link
Contributor Author

Added to changelog along with categories for bugs/features/fixes on master (unreleased)

@bbatsov bbatsov merged commit cdff72e into clojure-emacs:master Jul 27, 2016
@dpsutton dpsutton deleted the carriage-returns branch November 27, 2016 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants