feat(pull-request): Add table formatting to pull requests list

[hamxabaig]

This PR adds nice table formatting to pull requests list. I have used cli-table to show the list in the table. This feature is opt in and can be turned on by adding output: 'table' in ~/.gh.json.

I was not able to use terminal-link package to show links in the table. It has some weird bug, it won't show it as link. I have tried rendering it outside and it works but inside the table cell it doesn't work. I think it has something to do with the cli-table encoding of strings.

I have used some package to show the pull request's body nicely in the terminal. As the description might have markdown.

So with the final push, it looks like this (its with --detailed flag):

According to the discussions as per #589

Testing PR long links:

https://github.com/tokilabs/gh/commit/dfc787a5fedb776cc22a3a7f4388767712b0e03b#diff-5b0afcc99bb531dfe82e0ff9b3d4b9bcR537

[protoEvangelion]

Thanks for the Christmas present @hamxabaig 🎁 ! This is looking great. I will take a look 😄

[hamxabaig]

Ah forgot to write @protoEvangelion Happy Christmas 🎄

Sure, let me know your thoughts when you review.

[protoEvangelion]

It looks fantastic! Nice job 😄 For the detailed flag however, some of the rows are not aligned properly.

For instance you can run: gh pr -u node-gh --detailed

[protoEvangelion]

Fixed it with:

  renderer: new TerminalRenderer({
    reflowText: true,
    width: length
})

But is is still having problems with links when set like this [link](google.com)

[protoEvangelion]

Opened an issue here to fix the terminal text links: https://github.com/Automattic/cli-table/issues/118

[hamxabaig]

You're right, its breaking with the links. Do you see something like this too?

[hamxabaig]

When i output the content of body in separate file and cat in terminal it shows fine. So i guess this is a problem with cli-table

[hamxabaig]

Okay i removed the cli-table package and used cli-table2 and it wraps them fine. Its rendering pretty nicely now:

[protoEvangelion]

Almost at the finish line!

As a frontend guy, I often think of "Is this responsive?". Though it works pretty well, at a certain point it becomes unusable (looks like about 75 cols).

I can think of two options:

1. the easy way: if process.stdout.colums < 75 then do not print as a table but print normally.
2. the hard but better way: create a 1 column layout similar to how you did for formatting the PR body.

Thoughts?

[hamxabaig]

I think we can go with second option. We still have many options to make other layouts. Check this out I think maybe something like (its one column with multiple rows)

[pull_number | author  | status]
[title]
[body]
[link]

As the number, status and author take less space so we can format in one row with many columns and rest can go with its own rows.

What do you think?

[protoEvangelion]

Yes I think that format is perfect! I pushed some commits that alter formatting a bit like making the #, status, author & date cols fixed and only the title col dynamic. Also modified the colors a little.

Let me know if that all still achieves what you are looking for.

Once you add the change for the responsive format we can go ahead and merge this 🎉

[hamxabaig]

@protoEvangelion Yeah definitely. I'm gonna work on it, been a bit busy as our company's beta release is near 😄

PS: Happy New Year 🎉

[protoEvangelion]

@hamxabaig I just rebased on top of a refactor of TypeScript and fixed the merge conflicts.

Nothing will change for you except for getting to use cooler syntax, installing the new dependencies & running npm run build or npm run build:watch 😄

Don't sweat writing types and all that. You can keep writing the JS you are used to 😉

[hamxabaig]

Cool, It was so difficult for me to type all that var stuff. I'm so used to ES6 😄

[protoEvangelion]

Haha I know how you feel! I'm workin on updating the syntax to es6 little by little and unesting the callback hell with async/await 🤞

[hamxabaig]

I have tried playing with it but its being broken somehow with --detailed tag

[hamxabaig]

I think it finally works now. I used percentages to define colSpan and it seems to be working fine. The mess up of table has something to do with the terminal being smaller. So i added some percentage to reduce the span.

[protoEvangelion]

Fantastic job on making this responsive! This is not a blocker but any idea why emojis might be breaking the columns in iTerm? I wasn't able to find anything that would cause that in the code or track down another issue like that on google.

[protoEvangelion]

Looks like it breaks every line there is an emoji

[hamxabaig]

@protoEvangelion Yeah i think its because of the calculations done by the table component. The emojis take unkown space when rendered. They just calculate the table borders using text but when its really rendered it takes more space then before, thats why it breaks.

I bet if there is any solution to that. Its gonna be very hard.

[protoEvangelion]

The weird thing is it works in my VS Code editor which I normally use. But it's not a big deal and we can move forward.

I added labels and some formatting as well.

If that works for you and there is nothing else, give me the green light and I will merge this awesome PR!

[hamxabaig]

Cool then lets merge so that people enjoy this listing format as much as i do 😄

[protoEvangelion]

Hmm Travis is failing with the gh pr --detailed cmd but it is passing locally. I will debug and release this as soon as I can 😄

[protoEvangelion]

So the issue is somehow related to utf8 encoding. The table characters like https://graphemica.com/%E2%94%98 are represented with 3 bytes but utf8 encodes characters with 1 byte (8 bits).

Travis is printing out three replacement symbols which means it can't figure out the correct symbol.

This is a brand new arena for me so if you have any insight into encoding, please chime in 🙏

[hamxabaig]

Hey, for me too 😄 However, I'll check on weekend if i can find anything.

[protoEvangelion]

:tada: This PR is included in version 1.14.0 :tada:

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot :package::rocket:

[protoEvangelion]

Wow that was challenging @hamxabaig . It ended up being that there were hidden ansi characters on Travis only! Anyways, thanks for your amazing work on this and incredible patience 🥇