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

Fix inherits #48

Merged
merged 8 commits into from
Oct 27, 2017
Merged

Fix inherits #48

merged 8 commits into from
Oct 27, 2017

Conversation

lelit
Copy link
Contributor

@lelit lelit commented Oct 26, 2017

Description

Currently when a table has children, the \d command outputs something like

Inherits: ('parent_table_a',),
        : ('parent_table_b',),

that is

  • it emits the whole record tuple, instead of just the table name
  • it always adds a trailing comma
  • it repeats the colon on every row

With this PR it would emit something like

Inherits: parent_table_a,
          parent_table_b

matching the output of the corresponding command in psql.

Checklist

  • I've added this contribution to the changelog.rst.

Emit only the table name, not the whole record tuple, and add the trailing comma only when needed.
This make it easier to pass additional arguments to pytest, for example to show the complete
diff in case of errors you would execute `tox -- -vv`.
@lelit
Copy link
Contributor Author

lelit commented Oct 26, 2017

Uhm, should I fix those style issues too?

@j-bennet
Copy link
Contributor

@lelit Looks like pep8radius is not happy with these changes.

@lelit
Copy link
Contributor Author

lelit commented Oct 26, 2017

Yes, I saw that, but strictly speaking those errors are not caused by my changes 😈 so I wonder if I should take care of them in the context of this PR or not.

@j-bennet
Copy link
Contributor

Yes, please do - it only takes running pep8radius master -i --docformatter anyway.

@lelit
Copy link
Contributor Author

lelit commented Oct 26, 2017

Ok, will do that tomorrow. Thank you.

@codecov-io
Copy link

codecov-io commented Oct 26, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@802fdad). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #48   +/-   ##
=========================================
  Coverage          ?   52.46%           
=========================================
  Files             ?        6           
  Lines             ?      911           
  Branches          ?        0           
=========================================
  Hits              ?      478           
  Misses            ?      433           
  Partials          ?        0
Impacted Files Coverage Δ
pgspecial/dbcommands.py 52.46% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 802fdad...b0c8b6a. Read the comment docs.

@lelit
Copy link
Contributor Author

lelit commented Oct 26, 2017

Well, I found some spare minutes... :)

results = executor('\d tbl1')
title = None
rows = [['id1', 'integer', ' not null'],
['txt1', 'text', ' not null'],
]
headers = ['Column', 'Type', 'Modifiers']
status = 'Indexes:\n "id_text" PRIMARY KEY, btree (id1, txt1)\n'
status = ('Indexes:\n "id_text" PRIMARY KEY, btree (id1, txt1)\n'
'Number of child tables: 2 (Use \\d+ to listthem.)\n')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to listthem" - is that a typo?

@j-bennet
Copy link
Contributor

Looks great! Merging.

@j-bennet j-bennet merged commit e73a336 into dbcli:master Oct 27, 2017
@lelit
Copy link
Contributor Author

lelit commented Oct 27, 2017

Great, thank you!

@lelit lelit deleted the fix-inherits branch October 27, 2017 16:31
@lelit lelit mentioned this pull request Oct 30, 2017
1 task
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

Successfully merging this pull request may close these issues.

3 participants