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

Please fix the ls situation. #813

Closed
envygeeks opened this issue Jan 16, 2013 · 20 comments
Closed

Please fix the ls situation. #813

envygeeks opened this issue Jan 16, 2013 · 20 comments

Comments

@envygeeks
Copy link
Contributor

I understand there is already a ticket open to improve the ls command but it would be nice if you guys could issue a hotfix (well as hot as gemcutter gets obviously) minor minor release to address the situation with ls because the lack of tabbing murders readability with sparse classes. It would be nice if you indent each line and (possibly) only \n if it has more than one or extends past the new term width.

As it stands right now, I think the new ls is a breaking behavior because it's absolutely unreadable.

@rking
Copy link
Contributor

rking commented Jan 17, 2013

+1. Let's compile a few "what we have" vs "what would be ideal" examples. Really, I think the current output was made for ls _pry_ and not much more.

@envygeeks
Copy link
Contributor Author

constants: 
ObjectExt  VERSION
Pry::VTermAliases.methods: 
aliases  create_aliases  run_command  term
instance variables: 
@aliases  @terminal

Looks better as:

constants: ObjectExt  VERSION
Pry::VTermAliases.methods: aliases  create_aliases  run_command  term
instance variables: @aliases  @terminal

But using the _pry_ example would look better as: (all methods are fake)

Pry#methods: 
  method1        method2        method3        method4         method5
  method6        method7        method8        method9        method10

instance variables: @var1 @var2

The idea is that if you have to split it up add an extra line so that it's easily noticeable but if you don't have to split it up the clarity isn't really much improved since the bold is easy to spot when it's all grouped up together like the "looks better as" example.

This is my personal opinion.

@rking
Copy link
Contributor

rking commented Jan 17, 2013

Here's a real-life example of the output basically punishing a class hierarchy for being decently organized (in that there are many classes with fewer methods):

pry-punishing-decent-organiztion

@rking
Copy link
Contributor

rking commented Jan 17, 2013

  • If you put stuff on the same line as the headings:
    • either you have to count the heading as a member of the column (making it very wide)
    • or you have complex logic, such as either displaying it all-as-one-line or as-a-block, depending
  • If you put the headings on separate lines, it does the punish-for-good-practices thing a bit

@rking
Copy link
Contributor

rking commented Jan 17, 2013

OK, so here's the 0.9.10 (previous release) output:

pry-0 9 10-ls

@rking
Copy link
Contributor

rking commented Jan 17, 2013

Here's feature/ls-output-tweaks:

pry-ls-proposal

What do you think?

@rking
Copy link
Contributor

rking commented Jan 17, 2013

Here it is with protected methods toned down more:

pry-ls-proposal

@ghost
Copy link

ghost commented Jan 17, 2013

@epitron's screenshot is the best I've seen so far.

@rking
Copy link
Contributor

rking commented Jan 17, 2013

@robgleeson What about it do you like?

@rking
Copy link
Contributor

rking commented Jan 17, 2013

For others' reference, this is the one we're talking about:

epitron.meth

@rking
Copy link
Contributor

rking commented Jan 17, 2013

We could throw arity and ancestry in, sure, but I'd want to do that more as a pry 0.9.12 goal than as a 0.9.11.3 goal.

@ghost
Copy link

ghost commented Jan 17, 2013

yeah, the arity is a nice touch but it also seems like it's the easiest one to read through.

@rking
Copy link
Contributor

rking commented Jan 17, 2013

It's apples and oranges, comparing Object vs Rails stuff. Here's my ls Object:

pry-ls-object-proposal

Which, that's not even the same as his twacked out one.

@envygeeks
Copy link
Contributor Author

Having arity in ls would be real annoying and it adds noise that can be gathered from show-source and friends.Thus far the best output is the last one that @rking put out, I even like the colouring because would be alright on both light and dark terminals.

@ghost
Copy link

ghost commented Jan 18, 2013

@envygeeks I disagree with you. I think the arity is a nice touch.

@envygeeks
Copy link
Contributor Author

@robgleeson Fair enough, but not if you force it on people as if everybody shares your opinion on noise.

@ghost
Copy link

ghost commented Jan 18, 2013

@envygeeks I'm not forcing my opinion on anybody. I'm just expressing my opinion.

@banister
Copy link
Member

@rking is this fixed yet? i pushed a new gem with some ls features, i assumed it related to this issue...?!

@rking
Copy link
Contributor

rking commented Jan 18, 2013

@banister I think so.

@envygeeks Do you think so?

If you mostly-like pry-0.9.11.3's output, but want some tweaks, please create a new (less urgent sounding) Issue and close+comment this one with a link to that one.

@envygeeks
Copy link
Contributor Author

The new version is absolutely superb and beautiful. So gonna close this ticket so that other people don't think it's an ongoing issue. Thanks @rking for fixing this issue so quickly.

havenwood pushed a commit to havenwood/pry that referenced this issue Nov 12, 2013
The previous table output was geared mostly for `ls _pry_`, which isn't
a common of hierarchy. After feedback from users such as @envygeeks, we
found a few tweaks that would help the really-small layers such as those
found in Rails or in small classes, namely:

- Rolling it up onto one line, if possible
- Highlighting the heading in the colors familiar to users of GNU ls for
  "directory" style

Additionally, I took the opportunity for toning down the
private/protected method colors, because before they were green and
yellow, now they're both "muted terminal blue"

Without the ability to really get in and really distinguish colors (e.g.
using 256 colors), giving "protected" such a loud color seems wrong.

Before recoloring:
  pry#813 (comment)
After:
  pry#813 (comment)
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

No branches or pull requests

3 participants