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

Minimize string formatting in format-spec #465

Merged
merged 1 commit into from
Jan 3, 2018

Conversation

xiongtx
Copy link
Member

@xiongtx xiongtx commented Jan 2, 2018

Part of clojure-emacs/cider#2150

Let CIDER take care of as much formatting as possible.

Part of clojure-emacs/cider#2150

Let CIDER take care of as much formatting as possible.
@bbatsov
Copy link
Member

bbatsov commented Jan 2, 2018

Doesn't changing those functions affect any tests?

@xiongtx
Copy link
Member Author

xiongtx commented Jan 2, 2018

Not as far as I can tell.

Unfortunately CI has been broken for some time](https://travis-ci.org/clojure-emacs/cider-nrepl/builds). But it seems the failures are not related to this change, but from:

FAIL in (step-in-to-function-in-current-project-test) (debug_integration_test.clj:495)
step in
Message: {:debug-value "\"bar\"", :original-ns "cider.nrepl.middleware.util.misc", :key "48460051-82d8-4ee1-9230-130e1794a36d", :locals [["x" "\"bar\""]], :file "file:/home/txx/github/cider-nrepl/src/cider/nrepl/middleware/util/misc.clj", :column 0, :input-type {:q "quit", :o "out", :n "next", :e "eval", :s "stacktrace", :l "locals", :c "continue", :j "inject", :h "here", :t "trace", :p "inspect", :i "in"}, :prompt [], :coor [3 1 1], :line 37, :status ["need-debug-input"], :id 62, :code "(defn as-sym\n  [x]\n  (cond\n    (symbol? x) x\n    (string? x) (if-let [[_ ns sym] (re-matches #\"(.+)/(.+)\" x)]\n                  (symbol ns sym)\n                  (symbol x))))", :original-id 65, :session "cb583e4b-4918-4504-94c1-1f7bc3dbd975"}
expected: (clojure.core/= 32 (clojure.core/get msg6989 :line))
  actual: (not (clojure.core/= 32 37))

You can see this from failures in previous builds.

@bbatsov
Copy link
Member

bbatsov commented Jan 2, 2018

Yeah, I've been meaning to look into this broken test for a while now, but had 0 time for this.

I actually meant something else - I was surprised that changing this didn't break any of this middleware's tests. First I thought you forgot about the tests, but I guess there were no tests for the formatted result.

@xiongtx
Copy link
Member Author

xiongtx commented Jan 2, 2018

The breaking test is fixed in #467.

@bbatsov bbatsov merged commit b1469f1 into clojure-emacs:master Jan 3, 2018
@bbatsov
Copy link
Member

bbatsov commented Jan 3, 2018

Thanks! 🙇

@xiongtx xiongtx deleted the simplify-format-spec branch January 3, 2018 06:43
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