Skip to content

Fix for issue #717 #727

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix for issue #717 #727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eee4cce
Fix for #717: Rename of conflicting 'action' attribute; fallback for …
xarx00 Mar 4, 2019
0d5d423
Fixed PEP8 error (E501 line too long)
xarx00 Mar 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions gitlab/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,13 @@ def main():
if args.fields:
fields = [x.strip() for x in args.fields.split(',')]
debug = args.debug
action = args.action
action = args.whaction
what = args.what

args = args.__dict__
# Remove CLI behavior-related args
for item in ('gitlab', 'config_file', 'verbose', 'debug', 'what', 'action',
'version', 'output'):
for item in ('gitlab', 'config_file', 'verbose', 'debug', 'what',
'whaction', 'version', 'output'):
args.pop(item)
args = {k: _parse_value(v) for k, v in args.items() if v is not None}

Expand Down
2 changes: 1 addition & 1 deletion gitlab/tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def test_parse_args(self):
parser = cli._get_parser(gitlab.v4.cli)
args = parser.parse_args(['project', 'list'])
self.assertEqual(args.what, 'project')
self.assertEqual(args.action, 'list')
self.assertEqual(args.whaction, 'list')

def test_parser(self):
parser = cli._get_parser(gitlab.v4.cli)
Expand Down
4 changes: 2 additions & 2 deletions gitlab/v4/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ def extend_parser(parser):

object_subparsers = object_group.add_subparsers(
title='action',
dest='action', help="Action to execute.")
dest='whaction', help="Action to execute.")
_populate_sub_parser_by_class(cls, object_subparsers)
object_subparsers.required = True

Expand Down Expand Up @@ -357,7 +357,7 @@ def display_dict(d, padding):
id = getattr(obj, obj._id_attr)
print('%s: %s' % (obj._id_attr.replace('_', '-'), id))
if hasattr(obj, '_short_print_attr'):
value = getattr(obj, obj._short_print_attr)
value = getattr(obj, obj._short_print_attr) or 'None'
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't None events just be skipped in the CLI?

target_title: None

target_title: None

target_title: None

target_title: None

target_title: None

target_title: None

target_title: None

target_title: None

target_title: None

Copy link
Contributor Author

@xarx00 xarx00 Mar 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I don't know what these bad events are. It seems to me better to display them then to hide them. Because I consider github as a tool that helps me to show precisely what's going on in GitLab. If it hides the bad results, it may hide an important clue to a problem I'm solving.

But I don't know how other parts of github behave. It's up to you to decide what you want it to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem I see with this issue is that there is no way to have a usable output without displaying multiple attributes. For events that would be target_type and action_name for exemple

What we could do (to be discussed):

  1. make the display_dict check for a repr method for the object. This method would return a key and a value to be displayed. We could use __repr__ ou __unicode__ for this maybe
  2. if the method doesn't exist, fallback to the _short_print_attr attribute
  3. if this attribute is None raise an error so we can fix the code and add the method from 1 (not sure about this step, it's not nice for end users).

Other idea: _short_print_attr could become a tuple, and we display multiple lines when needed

value = value.replace('\r', '').replace('\n', ' ')
# If the attribute is a note (ProjectCommitComment) then we do
# some modifications to fit everything on one line
Expand Down