node-gh / gh

(DEPRECATED) GitHub CLI made with NodeJS. Use the official https://cli.github.com/ instead.
http://nodegh.io
Other
1.71k stars 217 forks source link

Enhancement: Show pull requests list in a nice table #589

Closed hamxabaig closed 5 years ago

hamxabaig commented 5 years ago

Hey there, excellent work by the way. I use it almost every other hour daily. There is one enhancement which can be made. I mostly use it to list opened PRs.

When looking at the PR's list, it seems you have to look around and find the data you need even if its right in front of you. Thats why i think it needs a little bit of redesign?

I was playing with this table component which seems very nice. This is what i came up with:

image

I think with a little redesign, its very straight forward and looks nice as compared to this:

image

Right now i'm using this table locally with gh, I'll be happy to provide a PR if that makes sense. Oh one more thing, it also supports terminal links using this terminal-link in supported terminals (I use iterm2) which is nice.

protoEvangelion commented 5 years ago

Hey @hamxabaig I really like this!

A couple questions first:

  1. How does this look when the terminal isn't full width? Does it resize nicely?
  2. What are your thoughts on backwards compatibility. I know some GH users have scripts which parse the output of cmds like gh pr. Maybe we could add this to the ~/.gh.json config as a key like table: true or output: 'table' output: 'string'
hamxabaig commented 5 years ago

@protoEvangelion

  1. Yep it does, I have given the widths for the columns in % which i then calculate based on the terminal's total columns. The cli-table2 component then wraps the text if it doesn't fill. image

There is some space left on the right side, so if i calculate the widths more accurately. It should be good with responsiveness of the terminal.

  1. About this, yes definitely. I was thinking of adding it like that so that people can turn it off and fallback to old list.
protoEvangelion commented 5 years ago

Nice! Was there a reason you chose to use cli-table2 over https://github.com/Automattic/cli-table ?

I just read the cli-table src code which seems really clean/minimal and it appears to have better maintenance. I think it would be a great idea as well to include the terminal link idea 👍

One implementation thing to take into account is if the user adds the --detailed flag.

That being said I would be happy to merge a PR from you on this. It's a great idea!

Make sure to follow this and let me know if you have any questions in the meantime 😄

hamxabaig commented 5 years ago

Yeah, cli-table2 offers more features. Like new lines wrapping, more flexible columns management. More advanced options

About --detailed flag, what do you suggest? I think we can show the body of the PR below title in the title column. Or we can render it completely different then the above. Like below:

Title Body Author, Time, Status

┌───────────────┐
│ greetings     │
├───────────────┤
│ greetings     │
├───────┬───────┤
│ hello │ howdy │
└───────┴───────┘

I'll play with it and will check what i come up with.

protoEvangelion commented 5 years ago

I would be OK with having the body right below the title just separated by 2 linebreaks and/or and new partition. Or whatever you think looks good. The url we could handle with just linking the pr number (which we should probably just include anyways without the --detailed flag).

protoEvangelion commented 5 years ago

:tada: This issue has been resolved in version 1.14.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

theist commented 5 years ago

Hi!

Just updated my gh and the new gh pr table format kicked in. Is there an optional parameter to see as it was before this? I have poor sight and keep a big font on the screen. As a result the previous version was clearer for me and easier to grep.

hamxabaig commented 5 years ago

@theist if you put output: text in json config file. I think it'll revert back to original text format. https://github.com/hamxabaig/gh/commit/0355a3006f59f25b09be89d874ed6a7dede1fd1d#diff-19fe9f14f395325dacf92342653af099R712

theist commented 5 years ago

@hamxabaig Nope, but you at least point me to the right place in the code, it seems the config option to control this now is a boolean called config.pretty_print so I was able to revert it back.

Thanks!

hamxabaig commented 5 years ago

Ah @theist i remember now, cool you found it!