Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

Created PorcelainTable class #31

Merged
merged 5 commits into from
Aug 3, 2016
Merged

Created PorcelainTable class #31

merged 5 commits into from
Aug 3, 2016

Conversation

liiight
Copy link
Contributor

@liiight liiight commented Aug 2, 2016

Added a class that'll emulate git --porcelain option, useful for stuff (at least I'll use it 😄 ).

from terminaltables.other_tables import PorcelainTable

table_data = [
    ['Heading1', 'Heading2'],
    ['row1 column1', 'row1 column2'],
    ['row2 column1', 'row2 column2'],
    ['row3 column1', 'row3 column2']
]
table = PorcelainTable(table_data)
print (table.table)

 Heading1     | Heading2     
 row1 column1 | row1 column2 
 row2 column1 | row2 column2 
 row3 column1 | row3 column2 

@coveralls
Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage decreased (-0.9%) to 98.925% when pulling 16b2b59 on liiight:master into 6f209ed on Robpol86:master.

@Robpol86
Copy link
Owner

Robpol86 commented Aug 2, 2016

Can you add tests to this, similar to the GitHub table tests?

@liiight
Copy link
Contributor Author

liiight commented Aug 2, 2016

Yeah, sure. Sorry for not doing this upfront. Also, I'll add the table to the main init.py

@coveralls
Copy link

coveralls commented Aug 3, 2016

Coverage Status

Coverage increased (+0.003%) to 99.785% when pulling 8dc71cc on liiight:master into 6f209ed on Robpol86:master.

@liiight
Copy link
Contributor Author

liiight commented Aug 3, 2016

Dunno why build failed, probably took too long.
I suggest triggering different build for each py version, it'll be faster (Travis can run simultaneously).

Also another thing.
Ideally PorcelainTable should not show any colors. I do this now in my soltuion using some partial hackery. Ideally, this should/could be baked in BaseTable using a allow_colors flag that can be checked when generating the table.
This however means coupling terminaltables with a color library (let's say, colorclass for example 😉 ). I'm more than willing to make that PR, just not sure if that's something you want/agree with.

@Robpol86
Copy link
Owner

Robpol86 commented Aug 3, 2016

Ugh, why did that break. I'll look into why ssh-add is prompting for a password for an ssh key with no password set. Your PR looks fine so I'll merge it.

I can't run multiple Travis jobs per build because Coveralls.io doesn't combine coverage from Travis and AppVeyor. I have to combine all coverage myself and then send it from Travis once.

The colors thing is a bit out of scope for terminaltables. If you want to remove colors I would do it in your application. Either regex strip it out before passing strings to terminaltables or use colorclass which can strip colors.

@Robpol86 Robpol86 merged commit 607c639 into Robpol86:master Aug 3, 2016
@liiight
Copy link
Contributor Author

liiight commented Aug 3, 2016

Thanks. I just check if I pass table type as porcelain and if so just not apply color:
https://github.com/Flexget/Flexget/pull/1307/files#diff-487b1eb870fb818194584ff5fe02f66cR511
There's another topic I'd like to discuss, word wrapping when needed. Not sure if this is the best forum though.

@Robpol86
Copy link
Owner

Robpol86 commented Aug 3, 2016

There's an issue for that: #5

@liiight
Copy link
Contributor Author

liiight commented Aug 4, 2016

Cool. Word wrap is very tricky as it involves many design decisions (which column to wrap, etc.)

When is a new version expected to be released?

@Robpol86
Copy link
Owner

Robpol86 commented Aug 4, 2016

Probably not for a long time unfortunately. I've got one or two other projects I want to work on before taking care of more issues here. Maybe in a month I can consider making a quick release.

@liiight
Copy link
Contributor Author

liiight commented Aug 4, 2016

Well with your coverage I'd consider just doing ci. I'll keep an eye out for release though, thanks!

@Robpol86
Copy link
Owner

FYI I've just released a new version.

@liiight
Copy link
Contributor Author

liiight commented Oct 17, 2016

nice, thanks for the heads up!

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

Successfully merging this pull request may close these issues.

3 participants