JunoLab / atom-ink

IDE toolkit for Atom
MIT License
228 stars 40 forks source link

Inline results - changes in tree, difficult to see results #69

Closed mauricioszabo closed 7 years ago

mauricioszabo commented 8 years ago

Hello, in the master branch (the one I'm using to make a PR for Ink) I've seen that inline results that contains trees became more difficult to read. This is some code, in Clojure, with the old tree:

image

and with the new one: notice as values get pushed to the end of list, and it's very difficult to inspect then:

image

Can we, at least, get a configuration parameter to decide which one to use? I need to use huge maps and vectors in Clojure, and with this new tree view it's completely unusable.

pfitzseb commented 8 years ago

Strange -- this looks good for me: trees

Do you have some custom css for your results or something like that?

jasongilman commented 8 years ago

This might be a problem from the Proto REPL side. I forked the Atom ink tree code in Proto REPL at first to improve performance and then to offer some new capabilities like buttons in the tree. I did not attempt to push that back to Atom ink. It's still using the styling from the Atom ink package though.

On Aug 17, 2016, at 11:11 AM, Maurício Szabo notifications@github.com wrote:

Hello, in the master branch (the one I'm using to make a PR for Ink) I've seen that inline results that contains trees became more difficult to read. This is some code, in Clojure, with the old tree:

and with the new one: notice as values get pushed to the end of list, and it's very difficult to inspect then:

Can we, at least, get a configuration parameter to decide which one to use? I need to use huge maps and vectors in Clojure, and with this new tree view it's completely unusable.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

pfitzseb commented 8 years ago

Our tree view code has changed a bit since then, so maybe that causes a mismatch between the css and the dom?

mauricioszabo commented 8 years ago

Well, I've created these screenshots opening Atom with the master version of Ink and proto-repl, and then checking out the 4.0.0 tag of Ink and opening another Atom instance with it, so no stylesheet issues nor any changes on proto-repl version.

pfitzseb commented 8 years ago

I'm relatively sure your issue happens because of this line -- can you try deleting that?

Not quite sure why it doesn't cause any issues for me though...

mauricioszabo commented 8 years ago

Yes, that worked fine. Deleting the line corrects the tree layout.

pfitzseb commented 8 years ago

Great, now we only need to find out why it's there (@MikeInnes) and why it's not causing any problems for us but breaks proto-repl ;)

jasongilman commented 8 years ago

I fixed the problem within Proto REPL. Would you be interested in getting the updated tree view pushed back to Ink from Proto REPL? https://github.com/jasongilman/proto-repl/blob/master/lib/tree-view.coffee#L56 It adds the ability to optionally display buttons like in the following picture:

demo_clj_ _src_ __users_jason_work_github_workspace_proto-repl-demo

I use the buttons to allow interaction with inline values that were displayed. Proto REPL can capture all the values from the middle of an algorithm. The added buttons allow the user to "def" one or more of the captured values (define a var with the same name as a local binding) so they can try executing some of the code to help debug an issue.

The buttons that are added can be of any name and used for any particular purpose.

MikeInnes commented 8 years ago

The change is there so that we can display tuples like this:

screenshot 2016-08-23 22 51 02

IIRC the fix is just to have an extra div around things that need to be on their own line, but sounds like it's been figured out on proto's end anyway.

@jasongilman thanks a lot for the offer! I'll take a look at the code and we can figure out what makes sense. As an immediate question, do you think it's possible to implement this by passing appropriate HTML into the tree view rather than by modifying tree-view.coffee itself? Also, how does it interact with horizontally-nested views like the above?

pfitzseb commented 7 years ago

I'll close this, since the original problem is fixed. Upstraming proto-repls modified code can be discussed in a separate issue (or even PR, if someone's up for it :)).