borgmatic-collective/borgmatic
borgmatic-collective
/
borgmatic
14
58
Fork 42
Code Issues 59 Pull Requests 3 Actions Packages Releases 122 Activity

Add space to separate comments from tokens #216

Manually merged
witten merged 1 commits from :comments-white-space into master 2019-09-18 21:03:56 +00:00
Conversation 4 Commits 1 Files Changed 2 +12 -12
2 changed files with 12 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
borgmatic/config/generate.py
View File

@ -45,10 +45,10 @@ def _comment_out_line(line):
# Comment out the names of optional sections.
one_indent = ' ' * INDENT
if not line.startswith(one_indent):
return '#' + line
return '# ' + line
witten commented 2019-09-18 16:31:24 +00:00
Review
Copy Link

So, if you want to deal with the blank line situation, you could do something like this:

    if not line:
        return '#'
    elif not line.startswith(one_indent):
        return '# ' + line
So, if you want to deal with the blank line situation, you could do something like this: ```python if not line: return '#' elif not line.startswith(one_indent): return '# ' + line ```
polyzen commented 2019-09-18 17:24:30 +00:00
Review
Copy Link

As there are no commented blank lines in the generated config, I thought the flexmock line in the tests had to be changed. Perhaps both?

As there are no commented blank lines in the generated config, I thought the flexmock line in the tests had to be changed. Perhaps both?
witten commented 2019-09-18 17:58:14 +00:00
Review
Copy Link

It's really up to you. I could see either changing the tests as you already did, or changing the code to handle the blank line special case. Just let me know!

It's really up to you. I could see either changing the tests as you already did, or changing the code to handle the blank line special case. Just let me know!
polyzen commented 2019-09-18 20:33:56 +00:00
Review
Copy Link

Making this change doesn't effect the outcome of the test, so I guess we're good to go here. 🤷

Making this change doesn't effect the outcome of the test, so I guess we're good to go here. 🤷
# Otherwise, comment out the line, but insert the "#" after the first indent for aesthetics.
return '#'.join((one_indent, line[INDENT:]))
return '# '.join((one_indent, line[INDENT:]))
REQUIRED_KEYS = {'source_directories', 'repositories', 'keep_daily'}

20
tests/integration/config/test_generate.py
View File

@ -31,17 +31,17 @@ def test_comment_out_line_skips_already_commented_out_line():
def test_comment_out_line_comments_section_name():
line = 'figgy-pudding:'
assert module._comment_out_line(line) == '#' + line
assert module._comment_out_line(line) == '# ' + line
def test_comment_out_line_comments_indented_option():
line = ' enabled: true'
assert module._comment_out_line(line) == ' #enabled: true'
assert module._comment_out_line(line) == ' # enabled: true'
def test_comment_out_optional_configuration_comments_optional_config_only():
flexmock(module)._comment_out_line = lambda line: '#' + line
flexmock(module)._comment_out_line = lambda line: '# ' + line
config = '''
foo:
bar:
@ -57,17 +57,17 @@ location:
'''
expected_config = '''
#foo:
# bar:
# - baz
# - quux
#
# foo:
# bar:
# - baz
# - quux
#
polyzen commented 2019-09-18 00:04:09 +00:00
Review
Copy Link

Is there somewhere you’re seeing a blank line get commented?

Here and below I had to do as such.

> Is there somewhere you’re seeing a blank line get commented? Here and below I had to do as such.
witten commented 2019-09-18 16:29:31 +00:00
Review
Copy Link

Ah, gotcha! Okay, if you want to prevent commented-out blank lines from ending up with a space after the #, you can change the commenting-out logic a bit. I'll add a note where you can do that above.

Ah, gotcha! Okay, if you want to prevent commented-out blank lines from ending up with a space after the `#`, you can change the commenting-out logic a bit. I'll add a note where you can do that above.
location:
repositories:
- one
- two
#
# other: thing
#
# other: thing
'''
assert module._comment_out_optional_configuration(config.strip()) == expected_config.strip()