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

Improve the presentation of nrepl messages #1726

Closed
bbatsov opened this issue Apr 30, 2016 · 14 comments
Closed

Improve the presentation of nrepl messages #1726

bbatsov opened this issue Apr 30, 2016 · 14 comments

Comments

@bbatsov
Copy link
Member

bbatsov commented Apr 30, 2016

The presentation of nREPL messages we currently use leaves a ton of room for improvement.

image

  • we lack proper font-locking that would make it easier to figure out what is what
  • even the standard message keys are displayed in different order in every message
  • values are not properly indented
  • sometimes several keys end up being displayed on the same line, which gets me every time (I just miss the second key)
@bbatsov
Copy link
Member Author

bbatsov commented Apr 30, 2016

This article might be useful for the font-locking code if anyone decides to tackle this, although we should get by just fine with some generic font-locking without the notion of keywords.

@Malabarba
Copy link
Member

Font locking in the sense of font-lock-mode shouldn't be necessary. We can just propertize a specific face to every other sexp.

@bbatsov
Copy link
Member Author

bbatsov commented Apr 30, 2016

True. I mostly want top-level keys to stand out. Bonus points for properly font-locked, strings, nils, etc.

@dpsutton
Copy link
Contributor

dpsutton commented May 5, 2016

Did @Malabarba 's commit fully satisfy the request? I find myself reinventing lots of things in cider only to find out that the necessary features already exist, there's a pattern to follow, etc. If yall would put a few pointers in here I'd be happy to run through and work on this.

@Malabarba
Copy link
Member

My commit only highlights the request's keys, which was the lowest-hanging of the fruits here. I feel that improves the readability significantly.

It would also be nice to have proper string font-locking and proper indentation, but these might be a little more difficult to do efficiently (without making the buffer supper slow) and they're really not high priority for me.

@bbatsov
Copy link
Member Author

bbatsov commented May 5, 2016

I'd say that ordering the request/response keys consistently should be relatively easy to do and would be very helpful as well. Tackling the bug with several keys getting displayed on the same line is also important.

@bbatsov
Copy link
Member Author

bbatsov commented Oct 9, 2016

@dpsutton This ticket is still yours if you want it. ;-)

@Malabarba
Copy link
Member

Given that this only really affects the cider developers, and even we weren't bothered enough to actually do something significant about it, maybe it's not that relevant. People's effort might be better spent elsewhere.

@bbatsov
Copy link
Member Author

bbatsov commented Oct 9, 2016

While this is true, this issue still represents an open thread and requires way less effort than most open tasks we have. I've been trying to do some cleanup lately, as tasks really piled on in the past 5-6 months. Hopefully contributor activity will pick up, as I'm pretty short on time myself.

@harold
Copy link
Contributor

harold commented Mar 8, 2017

After goofing around here for a bit, another potential improvement would be lining things up:

(-->
  code     "(defn map-sequence->dataset "Turns an infinite training se..."
  column   1
  file     "/home/harold/src/cortex/src/cortex/dataset.clj"
  id       "45"
  line     377
  ns       "cortex.dataset"
  op       "eval"
  session  "499ca8d8-43c8-40bf-aa19-ff369673ce4c"
)

cf. the extant

(-->
  code  "(defn map-sequence->dataset "Turns an infinite training se..."
  column  1
  file  "/home/harold/src/cortex/src/cortex/dataset.clj"
  id  "45"
  line  377
  ns  "cortex.dataset"
  op  "eval"
  session  "499ca8d8-43c8-40bf-aa19-ff369673ce4c"
)

@bbatsov
Copy link
Member Author

bbatsov commented Mar 8, 2017

Yeah, that'd be pretty great and pretty easy as well.

@harold
Copy link
Contributor

harold commented Mar 9, 2017

Updated the PR with an additional commit.

@dpsutton
Copy link
Contributor

dpsutton commented Mar 9, 2017

the build failed with

nrepl-client.el:1324:1:Error: the function ‘copy-list’ is not known to be defined

image

@harold
Copy link
Contributor

harold commented Mar 10, 2017

@dpsutton I believe that's fixed in the latest commit. See the discussion on the PR.

bbatsov pushed a commit that referenced this issue Mar 10, 2017
In order to have the keys print in a predictable order, we can sort them before printing.

Some common keys are treated specially and are always displayed before the rest. This commit also improves the alignment of nrepl message values.
@bbatsov bbatsov closed this as completed Jan 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants