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

Make WITH expressions more readable #45 #46

Merged
merged 7 commits into from
Nov 15, 2019

Conversation

pombredanne
Copy link
Member

This implements a solution for #45

pombredanne and others added 5 commits November 14, 2019 16:45
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
This add a new render_as_readable() function that will wrap the WITH
expression in parens (unless this is only a simple WITH expression)

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Python 3.8 emits a SyntaxWarning for a string with an anomalous
backslash, e.g.:

  <unknown>:127: SyntaxWarning: invalid escape sequence \s

Fix by adding the missing 'r' prefix.

Change-Id: If9ad77c1e1707fa2432c8fc31eeb5e6d2d7200a5
Signed-off-by: Peter Kolbus <peter.kolbus@garmin.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Copy link
Contributor

@steven-esser steven-esser left a comment

Choose a reason for hiding this comment

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

LGTM, other than the small doc correction.

def render(self, template='{symbol.key}', *args, **kwargs):
def render(self, template='{symbol.key}', wrap_with_in_parens=False, *args, **kwargs):
"""
Return a formatted WITH expression. If `wrap_in_parens`, wrap in parens.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think wrap_in_parens should be wrap_with_in_parens to correctly mirror the argument name.

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed a commit to fix this.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Reported-by: Steven Esser <sesser@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@pombredanne pombredanne force-pushed the 45-more-readable-expressions branch from 2899cd4 to 651c48d Compare November 14, 2019 22:24
Copy link
Contributor

@steven-esser steven-esser left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@steven-esser steven-esser merged commit 0edacd7 into master Nov 15, 2019
@pombredanne pombredanne deleted the 45-more-readable-expressions branch May 10, 2021 17:42
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