Skip to content
This repository has been archived by the owner on Nov 4, 2022. It is now read-only.

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

Closed
hamxabaig opened this issue Dec 17, 2018 · 10 comments
Closed

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

hamxabaig opened this issue Dec 17, 2018 · 10 comments
Labels

Comments

@hamxabaig
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link
Member

protoEvangelion commented Dec 17, 2018

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
Copy link
Contributor Author

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
Copy link
Member

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).

hamxabaig added a commit to hamxabaig/gh that referenced this issue Dec 24, 2018
hamxabaig added a commit to hamxabaig/gh that referenced this issue Dec 24, 2018
hamxabaig added a commit to hamxabaig/gh that referenced this issue Dec 29, 2018
protoEvangelion pushed a commit to protoEvangelion/gh that referenced this issue Dec 29, 2018
protoEvangelion pushed a commit to protoEvangelion/gh that referenced this issue Dec 29, 2018
protoEvangelion pushed a commit to protoEvangelion/gh that referenced this issue Dec 29, 2018
protoEvangelion pushed a commit to protoEvangelion/gh that referenced this issue Dec 29, 2018
protoEvangelion pushed a commit to hamxabaig/gh that referenced this issue Jan 5, 2019
protoEvangelion pushed a commit to hamxabaig/gh that referenced this issue Jan 15, 2019
protoEvangelion pushed a commit to hamxabaig/gh that referenced this issue Jan 15, 2019
protoEvangelion pushed a commit to hamxabaig/gh that referenced this issue Feb 5, 2019
protoEvangelion pushed a commit to hamxabaig/gh that referenced this issue Feb 5, 2019
protoEvangelion pushed a commit that referenced this issue Feb 5, 2019
# [1.14.0](v1.13.9...v1.14.0) (2019-02-05)

### Features

* **pull-request:** Add table formatting to pull requests list ([5b1f35b](5b1f35b)), closes [#589](#589)
* **pull-request:** Add table formatting to pull requests list ([1312f97](1312f97)), closes [#589](#589)
* **pull-request:** Replace cli-table with cli-table2 & Improve table formatting ([44373e4](44373e4)), closes [#589](#589)
* **pull-request:** Replace cli-table with cli-table2 & Improve table formatting ([026b657](026b657)), closes [#589](#589)
* **pull-request:** switch to 1 column when <75 cols ([92ffac7](92ffac7))
* **pull-request:** use percentage to calculate colSpan ([9f6fefc](9f6fefc))
@protoEvangelion
Copy link
Member

🎉 This issue has been resolved in version 1.14.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@theist
Copy link

theist commented Jul 2, 2019

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
Copy link
Contributor Author

@theist if you put output: text in json config file. I think it'll revert back to original text format. hamxabaig@0355a30#diff-19fe9f14f395325dacf92342653af099R712

@theist
Copy link

theist commented Jul 2, 2019

@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
Copy link
Contributor Author

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants