coala-quickstart.py: Change the printing of bears #227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@gitmate-bot rebase |
|
Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently |
|
Automated rebase with GitMate.io was successful! 🎉 |
Travis tests have failedHey @tallicamike, 3rd Buildpytest |
ishanSrt
reviewed
May 25, 2018
coala_quickstart/generation/Bears.py
Outdated
| @@ -6,7 +6,7 @@ | |||
|
|
|||
| from coala_quickstart.Constants import ( | |||
| IMPORTANT_BEAR_LIST, ALL_CAPABILITIES, DEFAULT_CAPABILTIES) | |||
| from coala_quickstart.Strings import BEAR_HELP | |||
| from coala_quickstart.Strings import (PRINT_BEARS, BEAR_HELP) | |||
There was a problem hiding this comment.
no need of paranthesis here
ishanSrt
reviewed
May 26, 2018
coala_quickstart/generation/Bears.py
Outdated
| """ | ||
| Prints the relevant bears in sections separated by language. | ||
| Prints both the usable and unusable, relevant bears | ||
| in sections separated by language. |
There was a problem hiding this comment.
"seperated by language?"
needs a language change over here 😉
ishanSrt
reviewed
May 26, 2018
coala_quickstart/Strings.py
Outdated
| PRINT_BEARS = {'unusable': | ||
| {'msg': """ | ||
| \nBased on the configuration options the following | ||
| bears have been identified to be unusable: |
There was a problem hiding this comment.
an extra message on how to make these bears usable too would be good over here
There was a problem hiding this comment.
indicating that these bears can be made usable if coala-quickstart is not run in non-interactive mode (not exactly this language)
There was a problem hiding this comment.
Ok, I have to change this, it sound better with the indications. Also, do you feel that using PRINT_BEARS as in the PR is an ok option? I feel that it somehow looks ugly (hacky), but on the other hand I feel that the other options either add duplicate code, or are less flexible?
There was a problem hiding this comment.
I don't think it looks ugly or hacky, for the same reason
on the other hand I feel that the other options either add duplicate code
ishanSrt
reviewed
May 26, 2018
coala_quickstart/Strings.py
Outdated
|
|
||
| PRINT_BEARS = {'unusable': | ||
| {'msg': """ | ||
| \nBased on the configuration options the following |
There was a problem hiding this comment.
remove the \n in the front of the messages as it is creating additional blank lines
|
@tallicamike this is a good patch although can't make out why you need an |
ishanSrt
reviewed
May 26, 2018
| @@ -2,6 +2,7 @@ | |||
| import logging | |||
| import os | |||
| import sys | |||
| from collections import OrderedDict | |||
There was a problem hiding this comment.
I don't see the use of ordered dict over here
There was a problem hiding this comment.
I used a OrderedDict there because I wanted to make sure that each time coala-quickstart is run, fristly the unused bears are printed and then the used bears. (I wanted a deterministic execution, and, for the user, if he doesn't want to check the full log he'll only get a glimpse at the ones that were chosen - I feel this is the most common case). Sounds ok? Or should I change it?
|
manual rebase is needed |
gitmate-bot
reviewed
May 31, 2018
coala_quickstart/generation/Bears.py
Outdated
| for label_bears in nonempty_label_bears: | ||
| printer.print(PRINT_BEARS[label_bears]['msg']) | ||
| for language in relevant_bears[label_bears]: | ||
| printer.print(" [" + language + "]", |
There was a problem hiding this comment.
You do not use the preferred quotation marks.
Origin: QuotesBear, Section: all.quotes.
The issue can be fixed by applying the following patch:
--- a/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
+++ b/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
@@ -236,7 +236,7 @@
for label_bears in nonempty_label_bears:
printer.print(PRINT_BEARS[label_bears]['msg'])
for language in relevant_bears[label_bears]:
- printer.print(" [" + language + "]",
+ printer.print(' [' + language + "]",
color=PRINT_BEARS[label_bears]["colors"][0])
for bear in relevant_bears[label_bears][language]:
printer.print(" " + bear.name,
gitmate-bot
reviewed
May 31, 2018
coala_quickstart/generation/Bears.py
Outdated
| for label_bears in nonempty_label_bears: | ||
| printer.print(PRINT_BEARS[label_bears]['msg']) | ||
| for language in relevant_bears[label_bears]: | ||
| printer.print(" [" + language + "]", |
There was a problem hiding this comment.
You do not use the preferred quotation marks.
Origin: QuotesBear, Section: all.quotes.
The issue can be fixed by applying the following patch:
--- a/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
+++ b/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
@@ -236,7 +236,7 @@
for label_bears in nonempty_label_bears:
printer.print(PRINT_BEARS[label_bears]['msg'])
for language in relevant_bears[label_bears]:
- printer.print(" [" + language + "]",
+ printer.print(" [" + language + ']',
color=PRINT_BEARS[label_bears]["colors"][0])
for bear in relevant_bears[label_bears][language]:
printer.print(" " + bear.name,
gitmate-bot
reviewed
May 31, 2018
coala_quickstart/generation/Bears.py
Outdated
| printer.print(PRINT_BEARS[label_bears]['msg']) | ||
| for language in relevant_bears[label_bears]: | ||
| printer.print(" [" + language + "]", | ||
| color=PRINT_BEARS[label_bears]["colors"][0]) |
There was a problem hiding this comment.
You do not use the preferred quotation marks.
Origin: QuotesBear, Section: all.quotes.
The issue can be fixed by applying the following patch:
--- a/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
+++ b/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
@@ -237,7 +237,7 @@
printer.print(PRINT_BEARS[label_bears]['msg'])
for language in relevant_bears[label_bears]:
printer.print(" [" + language + "]",
- color=PRINT_BEARS[label_bears]["colors"][0])
+ color=PRINT_BEARS[label_bears]['colors'][0])
for bear in relevant_bears[label_bears][language]:
printer.print(" " + bear.name,
color=PRINT_BEARS[label_bears]["colors"][1])
gitmate-bot
reviewed
May 31, 2018
coala_quickstart/generation/Bears.py
Outdated
| printer.print(" [" + language + "]", | ||
| color=PRINT_BEARS[label_bears]["colors"][0]) | ||
| for bear in relevant_bears[label_bears][language]: | ||
| printer.print(" " + bear.name, |
There was a problem hiding this comment.
You do not use the preferred quotation marks.
Origin: QuotesBear, Section: all.quotes.
The issue can be fixed by applying the following patch:
--- a/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
+++ b/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
@@ -239,7 +239,7 @@
printer.print(" [" + language + "]",
color=PRINT_BEARS[label_bears]["colors"][0])
for bear in relevant_bears[label_bears][language]:
- printer.print(" " + bear.name,
+ printer.print(' ' + bear.name,
color=PRINT_BEARS[label_bears]["colors"][1])
printer.print("")
gitmate-bot
reviewed
May 31, 2018
coala_quickstart/generation/Bears.py
Outdated
| color=PRINT_BEARS[label_bears]["colors"][0]) | ||
| for bear in relevant_bears[label_bears][language]: | ||
| printer.print(" " + bear.name, | ||
| color=PRINT_BEARS[label_bears]["colors"][1]) |
There was a problem hiding this comment.
You do not use the preferred quotation marks.
Origin: QuotesBear, Section: all.quotes.
The issue can be fixed by applying the following patch:
--- a/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
+++ b/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
@@ -240,7 +240,7 @@
color=PRINT_BEARS[label_bears]["colors"][0])
for bear in relevant_bears[label_bears][language]:
printer.print(" " + bear.name,
- color=PRINT_BEARS[label_bears]["colors"][1])
+ color=PRINT_BEARS[label_bears]['colors'][1])
printer.print("")
gitmate-bot
reviewed
May 31, 2018
coala_quickstart/generation/Bears.py
Outdated
| for bear in relevant_bears[label_bears][language]: | ||
| printer.print(" " + bear.name, | ||
| color=PRINT_BEARS[label_bears]["colors"][1]) | ||
| printer.print("") |
There was a problem hiding this comment.
You do not use the preferred quotation marks.
Origin: QuotesBear, Section: all.quotes.
The issue can be fixed by applying the following patch:
--- a/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
+++ b/tmp/tmpswjp7wec/coala_quickstart/generation/Bears.py
@@ -241,7 +241,7 @@
for bear in relevant_bears[label_bears][language]:
printer.print(" " + bear.name,
color=PRINT_BEARS[label_bears]["colors"][1])
- printer.print("")
+ printer.print('')
def generate_requirements_map(bears):
gitmate-bot
reviewed
May 31, 2018
tests/generation/Bears.py
Outdated
| [('Python', 70), ('Unknown', 30)], self.printer, | ||
| self.arg_parser, {})) | ||
| print_relevant_bears(self.printer, OrderedDict([('unusable', {}), | ||
| ('usable', filter_relevant_bears([('Python', 70), ('Unknown', 30)], |
There was a problem hiding this comment.
The code does not comply to PEP8.
Origin: PEP8Bear, Section: all.autopep8.
The issue can be fixed by applying the following patch:
--- a/tmp/tmpswjp7wec/tests/generation/Bears.py
+++ b/tmp/tmpswjp7wec/tests/generation/Bears.py
@@ -288,8 +288,8 @@
def test_print_relevant_bears(self):
with retrieve_stdout() as custom_stdout:
print_relevant_bears(self.printer, OrderedDict([('unusable', {}),
- ('usable', filter_relevant_bears([('Python', 70), ('Unknown', 30)],
- self.printer, self.arg_parser, {}))]))
+ ('usable', filter_relevant_bears([('Python', 70), ('Unknown', 30)],
+ self.printer, self.arg_parser, {}))]))
self.assertIn("PycodestyleBear", custom_stdout.getvalue())
# Should print only the usable bears
self.assertNotIn("unusable",
gitmate-bot
reviewed
May 31, 2018
tests/generation/Bears.py
Outdated
| self.arg_parser, {})) | ||
| print_relevant_bears(self.printer, OrderedDict([('unusable', {}), | ||
| ('usable', filter_relevant_bears([('Python', 70), ('Unknown', 30)], | ||
| self.printer, self.arg_parser, {}))])) |
There was a problem hiding this comment.
E128 continuation line under-indented for visual indent
Origin: PycodestyleBear (E128), Section: all.autopep8.
gitmate-bot
reviewed
May 31, 2018
tests/generation/Bears.py
Outdated
| def test_print_relevant_bears_no_bears(self): | ||
| with retrieve_stdout() as custom_stdout: | ||
| print_relevant_bears(self.printer, OrderedDict([('unusable', {}), | ||
| ('usable', {})])) |
There was a problem hiding this comment.
E128 continuation line under-indented for visual indent
Origin: PycodestyleBear (E128), Section: all.autopep8.
gitmate-bot
reviewed
May 31, 2018
tests/generation/Bears.py
Outdated
| self.assertNotIn("usable", | ||
| # Should print only the usable bears | ||
| self.assertIn("usable", | ||
| custom_stdout.getvalue()) |
There was a problem hiding this comment.
E127 continuation line over-indented for visual indent
Origin: PycodestyleBear (E127), Section: all.autopep8.
gitmate-bot
reviewed
May 31, 2018
tests/generation/Bears.py
Outdated
| [('Python', 70), ('Unknown', 30)], self.printer, | ||
| self.arg_parser, {})) | ||
| print_relevant_bears(self.printer, OrderedDict([('unusable', {}), | ||
| ('usable', filter_relevant_bears([('Python', 70), ('Unknown', 30)], |
There was a problem hiding this comment.
Line is longer than allowed. (83 > 80)
Origin: LineLengthBear, Section: all.linelength.
Travis tests have failedHey @tallicamike, 3rd Buildcoala --non-interactive |
tallicamike
commented
May 31, 2018
coala_quickstart/generation/Bears.py
Outdated
| printer.print(BEAR_HELP) | ||
|
|
||
| # Don't print anything for empty bear sets | ||
| nonempty_label_bears = ( |
There was a problem hiding this comment.
Should we print something if there are no relevant bears?
There was a problem hiding this comment.
i am sure that would never be the case 😉
There was a problem hiding this comment.
yeah, imo a simple print statement saying no relevant bears found should suffice (just in case there's a change of design in future which actually allows no relevant bears, we don't want user to be confused when he sees an empy screen with no message printed)
ishanSrt
reviewed
May 31, 2018
coala_quickstart/Strings.py
Outdated
|
|
||
| PRINT_BEARS = {'unusable': | ||
| {'msg': """ | ||
| Based on the configuration options the following bears have been identified |
ishanSrt
reviewed
May 31, 2018
coala_quickstart/Strings.py
Outdated
| PRINT_BEARS = {'unusable': | ||
| {'msg': """ | ||
| Based on the configuration options the following bears have been identified | ||
| as unusable ('-C' or '--ci' set non-interactive mode, making the bears that |
ishanSrt
reviewed
May 31, 2018
coala_quickstart/Strings.py
Outdated
| Based on the configuration options the following bears have been identified | ||
| as unusable ('-C' or '--ci' set non-interactive mode, making the bears that | ||
| need to be configured by the user unusable: check '--allow-incomplete-sections' | ||
| for more information): |
There was a problem hiding this comment.
or don't run in non interactive mode
ishanSrt
reviewed
May 31, 2018
tests/generation/Bears.py
Outdated
| print_relevant_bears(self.printer, OrderedDict([('unusable', {}), | ||
| ('usable', {})])) | ||
| self.assertNotIn("usable", custom_stdout.getvalue()) | ||
| # Should print only the usable bears |
There was a problem hiding this comment.
no output is being printed in this case right?
this comment seems misleading because of that.
ishanSrt
reviewed
Jun 5, 2018
coala_quickstart/Strings.py
Outdated
| PRINT_BEARS = {'unusable': | ||
| {'msg': """ | ||
| Based on the configuration options the following bears have been identified | ||
| as unusable (options '-C' or '--ci' (non interactive mode) make the bears that |
There was a problem hiding this comment.
language trouble, parenthesis shouldn't be here starting before the options
There was a problem hiding this comment.
imo, the sentence is too long too, needs to split.
There was a problem hiding this comment.
currently, there's nesting of braces, and the sentence inside the braces is long too.
There was a problem hiding this comment.
How about this: "The following bears have been dropped because '-C'/'--ci' (non interactive mode) option has ben selected. Remove the option or check '--allow-incomplete-sections' option for more information on how to include them:" ?
ishanSrt
approved these changes
Jun 5, 2018
satwikkansal
requested changes
Jun 17, 2018
coala_quickstart/Strings.py
Outdated
| @@ -50,3 +50,18 @@ | |||
|
|
|||
| You can see all of them here: {} | |||
| """.format(BEAR_DOCS_URL) | |||
|
|
|||
| PRINT_BEARS = {'unusable': | |||
| {'msg': """ | |||
There was a problem hiding this comment.
A slight indentation issue, can you move the opening brace to previous line
PRINT_BEARS = {'unusable': {
coala_quickstart/Strings.py
Outdated
| PRINT_BEARS = {'unusable': | ||
| {'msg': """ | ||
| Based on the configuration options the following bears have been identified | ||
| as unusable (options '-C' or '--ci' (non interactive mode) make the bears that |
There was a problem hiding this comment.
imo, the sentence is too long too, needs to split.
coala_quickstart/Strings.py
Outdated
| PRINT_BEARS = {'unusable': | ||
| {'msg': """ | ||
| Based on the configuration options the following bears have been identified | ||
| as unusable (options '-C' or '--ci' (non interactive mode) make the bears that |
There was a problem hiding this comment.
currently, there's nesting of braces, and the sentence inside the braces is long too.
coala_quickstart/Strings.py
Outdated
| Based on the configuration options the following bears have been identified | ||
| as unusable (options '-C' or '--ci' (non interactive mode) make the bears that | ||
| need to be configured by the user unusable: don't run in non interactive mode | ||
| or check '--allow-incomplete-sections' for more information on how to include |
There was a problem hiding this comment.
"or check '--allow-incomplete-sections' option for more information...
There was a problem hiding this comment.
add "option" to it, that is?
coala_quickstart/Strings.py
Outdated
| or check '--allow-incomplete-sections' for more information on how to include | ||
| the unusable bears): | ||
| """, | ||
| 'colors': ('green', 'red')}, |
There was a problem hiding this comment.
There seems like an indentation issue in the entire dict, can you please check that?
There was a problem hiding this comment.
The linter isn't complaining, but indeed it is hard to read. Re-writing it nicely kind of beats me, what I would suggest would be to add variables for each 'msg' value of the dictionary so it should be more "readable". (or another option would be to create variables for the inner dictionaries - 'unusable' and 'usable')
I'm thinking of something like this:
PRINT_BEARS = {
'unusable': {
'msg': PRINT_UNUSABLE,
'colors': ('green', 'red')
},
What are your thoughts on this?
| print_relevant_bears(printer, relevant_bears) | ||
|
|
||
| # Drop unusable bears | ||
| relevant_bears = relevant_bears['usable'] |
There was a problem hiding this comment.
Isn't the remove_unusable_bears method doing this?
There was a problem hiding this comment.
There was a problem hiding this comment.
I changed remove_unusable_bears to also add the unusable bears to the OrderdDict, but I'm using that only for printing purposes. Here the printing has ended, so I drop unusable bears from the dict.
There was a problem hiding this comment.
I changed remove_unusable_bears to also add the unusable bears to the OrderdDict
doesn't that contradict with the function name? remove_unusable_bears is adding unusable bears to OrdereDict
coala_quickstart/coala_quickstart.py
Outdated
| print_relevant_bears(printer, relevant_bears) | ||
| # OrderedDict used for print_relevant_bears to first print unusable bears | ||
| relevant_bears = OrderedDict( | ||
| [('unusable', {}), |
There was a problem hiding this comment.
Not sure if I full understand this, where are we populating the dictionary with key unusable 😅
There was a problem hiding this comment.
see remove_unusable_bears in this PR
coala_quickstart/generation/Bears.py
Outdated
| printer.print(BEAR_HELP) | ||
|
|
||
| # Don't print anything for empty bear sets | ||
| nonempty_label_bears = ( |
There was a problem hiding this comment.
yeah, imo a simple print statement saying no relevant bears found should suffice (just in case there's a change of design in future which actually allows no relevant bears, we don't want user to be confused when he sees an empy screen with no message printed)
coala_quickstart/generation/Bears.py
Outdated
| @@ -200,38 +200,48 @@ def get_non_optional_settings_bears(bears): | |||
|
|
|||
| def remove_unusable_bears(bears, unusable_bears): | |||
There was a problem hiding this comment.
I think based on the new changes, a better name could be filter_unsuable_bears
There was a problem hiding this comment.
Oh, you are right! I didn't thought about that! Makes more sense now! Thank you!
Remove duplicate printing of `relevant_bears` and instead change `print_relevant_bears` to inform the user both of the usable and unusable bears. Closes coala#220
jayvdb
requested changes
Aug 1, 2018
| printer.print('') | ||
| printer.print(BEAR_HELP) | ||
|
|
||
| nonempty_label_bears = [ |
|
|
||
| if not nonempty_label_bears: | ||
| printer.print('No relevant bears were found.') | ||
| else: |
There was a problem hiding this comment.
you can use return here if there are no bears to print
then the following can be dedented
| PRINT_BEARS = { | ||
| 'unusable': { | ||
| 'msg': PRINT_UNUSABLE, | ||
| 'colors': ('green', 'red') |
|
|
||
| print_relevant_bears(printer, relevant_bears) | ||
| # OrderedDict used for print_relevant_bears to first print unusable bears | ||
| relevant_bears = OrderedDict( |
There was a problem hiding this comment.
labelled_bears = {'usable': selected_bears}
print_labels = ['usable']| @@ -103,18 +104,27 @@ def main(): | |||
|
|
|||
| extracted_information = collect_info(project_dir) | |||
|
|
|||
| relevant_bears = filter_relevant_bears( | |||
| selected_bears = filter_relevant_bears( | |||
| @@ -201,40 +201,52 @@ def get_non_optional_settings_bears(bears): | |||
| return non_optional_settings | |||
|
|
|||
|
|
|||
| def remove_unusable_bears(bears, unusable_bears): | |||
| def filter_unusable_bears(bears, unusable_bears): | |||
There was a problem hiding this comment.
This can stay as remove_unusable_bears , and return the removed bears grouped by language.
| for bear in tuple(language_bears): | ||
| if bear in unusable_bears: | ||
| bears[language].remove(bear) | ||
| bears['usable'][language].remove(bear) | ||
| bears['unusable'][language] = bears['unusable'].get( |
There was a problem hiding this comment.
return a new dict with the unusable bears grouped by language.
|
|
||
|
|
||
| def print_relevant_bears(printer, relevant_bears, label='relevant'): | ||
| def print_relevant_bears(printer, relevant_bears): |
There was a problem hiding this comment.
change this to:
def print_bears(printer, labelled_bears, labels=['usable']):
The order of the labels is then used to determine which groups to show.
| print_relevant_bears(printer, relevant_bears, 'usable') | ||
| unusable_bears = get_non_optional_settings_bears( | ||
| relevant_bears['usable']) | ||
| filter_unusable_bears(relevant_bears, unusable_bears) |
There was a problem hiding this comment.
labelled_bears['unusable'] = remove_unusable_bears(relevant_bears, unusable_bears)
print_labels = ['unusable', 'usable']| relevant_bears['usable']) | ||
| filter_unusable_bears(relevant_bears, unusable_bears) | ||
|
|
||
| print_relevant_bears(printer, relevant_bears) |
There was a problem hiding this comment.
print_labelled_bears(print, labelled_bears, print_labels)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove duplicate printing of
relevant_bears`` and instead changeprint_relevant_bears` to inform the user both of the usable andunusable bears.
Closes #220