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.

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

Jump to bottom

Allow specifying --diff for ansible-pull #37645

Merged
merged 1 commit into from
Aug 28, 2018
Merged

Allow specifying --diff for ansible-pull #37645

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/ansible/cli/pull.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import time

from ansible.cli import CLI
from ansible import constants as C
from ansible.errors import AnsibleOptionsError
from ansible.module_utils._text import to_native, to_text
from ansible.plugins.loader import module_loader
Expand Down Expand Up @@ -91,6 +92,7 @@ def parse(self):
vault_opts=True,
runtask_opts=True,
subset_opts=True,
check_opts=False, # prevents conflict of --checkout/-C and --check/-C
inventory_opts=True,
module_opts=True,
runas_prompt_opts=True,
Expand Down Expand Up @@ -123,8 +125,12 @@ def parse(self):
help='modified files in the working repository will be discarded')
self.parser.add_option('--track-subs', dest='tracksubs', default=False, action='store_true',
help='submodules will track the latest changes. This is equivalent to specifying the --remote flag to git submodule update')
# add a subset of the check_opts flag group manually, as the full set's
# shortcodes conflict with above --checkout/-C
self.parser.add_option("--check", default=False, dest='check', action='store_true',
help="don't make any changes; instead, try to predict some of the changes that may occur")
self.parser.add_option("--diff", default=C.DIFF_ALWAYS, dest='diff', action='store_true',
help="when changing (small) files and templates, show the differences in those files; works great with --check")
Copy link
Member

Choose a reason for hiding this comment

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

tempted to say just use check_opts=True and remove syntax check afterward with remove_option (we do this in ansible-inventory).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we did that, we'd have to remove --check too and and re-add it without it's shorthand -C? Instead of check_opts=False with two adds we'd end up with a True and two removals and one re-add. I suppose it would reduce the number of duplicate help strings by one. You prefer the second option? Then I'll invert the current PR to do it that way.

Copy link
Member

Choose a reason for hiding this comment

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

if it were not for the -C i would not say 'tempted' but actually request it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll consider this merge-ready then.


super(PullCLI, self).parse()

Expand Down Expand Up @@ -276,6 +282,8 @@ def run(self):
cmd += ' -l "%s"' % limit_opts
if self.options.check:
cmd += ' -C'
if self.options.diff:
cmd += ' -D'

os.chdir(self.options.dest)

Expand Down