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

Globbing.py: Convert glob '*' to '**' #5114

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ishanSrt
Copy link
Contributor

@ishanSrt ishanSrt commented Jan 26, 2018

Some bears' result.origin have os.sep in them which makes
the glob * not match against them. This commit removes the
implementation of this glob altogether and converts it into **.

Closes #5041

@@ -138,6 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
print("SAY WHAT?")
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp3so0n3am/coalib/processes/Processing.py
+++ b/tmp/tmp3so0n3am/coalib/processes/Processing.py
@@ -138,7 +138,7 @@
             action = default_actions[result.origin]
         except KeyError:
             for bear_glob in default_actions:
-                print("SAY WHAT?")
+                print('SAY WHAT?')
                 print("bear_glob:", bear_glob)
                 if fnmatch(result.origin, bear_glob):
                     action = default_actions[bear_glob]

@@ -138,6 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
print("SAY WHAT?")
print("bear_glob:", bear_glob)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp3so0n3am/coalib/processes/Processing.py
+++ b/tmp/tmp3so0n3am/coalib/processes/Processing.py
@@ -139,7 +139,7 @@
         except KeyError:
             for bear_glob in default_actions:
                 print("SAY WHAT?")
-                print("bear_glob:", bear_glob)
+                print('bear_glob:', bear_glob)
                 if fnmatch(result.origin, bear_glob):
                     action = default_actions[bear_glob]
                     break

@@ -170,6 +171,8 @@ def collect_bears(bear_dirs, bear_globs, kinds, log_printer=None,
:return: Tuple of list of matching bear classes based on
kind. The lists are in the same order as kinds.
"""
print("BEAR_GLOBS:", bear_globs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp3so0n3am/coalib/collecting/Collectors.py
+++ b/tmp/tmp3so0n3am/coalib/collecting/Collectors.py
@@ -171,7 +171,7 @@
     :return:                    Tuple of list of matching bear classes based on
                                 kind. The lists are in the same order as kinds.
     """
-    print("BEAR_GLOBS:", bear_globs)
+    print('BEAR_GLOBS:', bear_globs)
     print("WHEEEEEEEEE!!!!!")
     bears_found = tuple([] for i in range(len(kinds)))
     bear_globs_with_bears = set()

@@ -170,6 +171,8 @@ def collect_bears(bear_dirs, bear_globs, kinds, log_printer=None,
:return: Tuple of list of matching bear classes based on
kind. The lists are in the same order as kinds.
"""
print("BEAR_GLOBS:", bear_globs)
print("WHEEEEEEEEE!!!!!")
Copy link
Collaborator

Choose a reason for hiding this comment

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

You do not use the preferred quotation marks.

Origin: QuotesBear, Section: python.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp3so0n3am/coalib/collecting/Collectors.py
+++ b/tmp/tmp3so0n3am/coalib/collecting/Collectors.py
@@ -172,7 +172,7 @@
                                 kind. The lists are in the same order as kinds.
     """
     print("BEAR_GLOBS:", bear_globs)
-    print("WHEEEEEEEEE!!!!!")
+    print('WHEEEEEEEEE!!!!!')
     bears_found = tuple([] for i in range(len(kinds)))
     bear_globs_with_bears = set()
     for bear, glob in icollect_bears(bear_dirs, bear_globs, kinds):

@gitmate-bot
Copy link
Collaborator

Comment on 4a436f1.

No newline found between shortlog and body at HEAD commit. Please add one.

Origin: GitCommitBear, Section: commit.

@ishanSrt ishanSrt force-pushed the globglob branch 3 times, most recently from 993beb5 to 09aba61 Compare January 26, 2018 14:32
@@ -212,9 +212,12 @@ def fnmatch(name, globs):
More than two or just one sequence can be given.
- '?': Matches any single character.
- '*': Matches everything but os.sep.
[NOTE: Not implementing this anymore,
converting it internally into '**']
Copy link
Member

Choose a reason for hiding this comment

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

Could you change it to '*' to be more precise?

matches = ['axb', 'ayb']
non_matches = ['aXbX', os.path.join('a', 'b')]
matches = ['axb', 'ayb', os.path.join('a', 'b')]
non_matches = ['aXbX']
Copy link
Member

Choose a reason for hiding this comment

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

Why do you have to change the position of os.path.join('a', 'b') from non_matches to matches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the commit body explains that

matches = ['axb', 'ayb']
non_matches = ['aXbX', os.path.join('a', 'b')]
matches = ['axb', 'ayb', os.path.join('a', 'b')]
non_matches = ['aXbX']
Copy link
Contributor

Choose a reason for hiding this comment

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

Then how will pattern a**b and a*b ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what you mean by that question

Copy link
Contributor

@newbazz newbazz left a comment

Choose a reason for hiding this comment

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

wont a**b change to a****b?

@ishanSrt
Copy link
Contributor Author

@newbazz that won't make any difference while comparing globs

@ishanSrt
Copy link
Contributor Author

wont a**b change to a****b?

I am thinking to change that behaviour anyways

@newbazz
Copy link
Contributor

newbazz commented Jan 27, 2018

Why any loop hole?

@ishanSrt
Copy link
Contributor Author

@newbazz 😆😂 this isn't a "loophole" at all since the behaviour won't change at all.

- '**': Matches everything.
"""
globs = (globs,) if isinstance(globs, str) else tuple(globs)
globs = tuple([glob.replace('*', '**') for glob in globs
if glob=='*'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp045hi3_o/coalib/parsing/Globbing.py
+++ b/tmp/tmp045hi3_o/coalib/parsing/Globbing.py
@@ -218,7 +218,7 @@
     """
     globs = (globs,) if isinstance(globs, str) else tuple(globs)
     globs = tuple([glob.replace('*', '**') for glob in globs
-                   if glob=='*'])
+                   if glob == '*'])
 
     if len(globs) == 0:
         return True

- '**': Matches everything.
"""
globs = (globs,) if isinstance(globs, str) else tuple(globs)
globs = tuple([glob.replace('*', '**') for glob in globs
if glob=='*'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

E225 missing whitespace around operator

Origin: PycodestyleBear (E225), Section: autopep8.

@ishanSrt ishanSrt force-pushed the globglob branch 2 times, most recently from 319d6d9 to bfe4a5a Compare January 28, 2018 04:24
@ishanSrt
Copy link
Contributor Author

The glob ** means 0 to any number of any characters so two of them also mean the same. Anyways I made the changes.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpyc35g06a/coalib/processes/Processing.py
+++ b/tmp/tmpyc35g06a/coalib/processes/Processing.py
@@ -139,7 +139,7 @@
         except KeyError:
             for bear_glob in default_actions:
                 if fnmatch(result.origin, bear_glob.replace('*', '**')
-                  if bear_glob=='*' else bear_glob):
+                           if bear_glob == '*' else bear_glob):
                     action = default_actions[bear_glob]
                     break
             else:

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E128 continuation line under-indented for visual indent

Origin: PycodestyleBear (E128), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E225 missing whitespace around operator

Origin: PycodestyleBear (E225), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpf_4_v7mu/coalib/processes/Processing.py
+++ b/tmp/tmpf_4_v7mu/coalib/processes/Processing.py
@@ -139,7 +139,7 @@
         except KeyError:
             for bear_glob in default_actions:
                 if fnmatch(result.origin, bear_glob.replace('*', '**')
-                  if bear_glob=='*' else bear_glob):
+                           if bear_glob == '*' else bear_glob):
                     action = default_actions[bear_glob]
                     break
             else:

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E128 continuation line under-indented for visual indent

Origin: PycodestyleBear (E128), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E225 missing whitespace around operator

Origin: PycodestyleBear (E225), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp11trd8u9/coalib/processes/Processing.py
+++ b/tmp/tmp11trd8u9/coalib/processes/Processing.py
@@ -139,7 +139,7 @@
         except KeyError:
             for bear_glob in default_actions:
                 if fnmatch(result.origin, bear_glob.replace('*', '**')
-                      if bear_glob=='*' else bear_glob):
+                           if bear_glob == '*' else bear_glob):
                     action = default_actions[bear_glob]
                     break
             else:

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E128 continuation line under-indented for visual indent

Origin: PycodestyleBear (E128), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E225 missing whitespace around operator

Origin: PycodestyleBear (E225), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpvjqbzma8/coalib/processes/Processing.py
+++ b/tmp/tmpvjqbzma8/coalib/processes/Processing.py
@@ -139,7 +139,7 @@
         except KeyError:
             for bear_glob in default_actions:
                 if fnmatch(result.origin, bear_glob.replace('*', '**')
-                        if bear_glob=='*' else bear_glob):
+                           if bear_glob == '*' else bear_glob):
                     action = default_actions[bear_glob]
                     break
             else:

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E128 continuation line under-indented for visual indent

Origin: PycodestyleBear (E128), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E225 missing whitespace around operator

Origin: PycodestyleBear (E225), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code does not comply to PEP8.

Origin: PEP8Bear, Section: autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpm_rvtf_q/coalib/processes/Processing.py
+++ b/tmp/tmpm_rvtf_q/coalib/processes/Processing.py
@@ -139,7 +139,7 @@
         except KeyError:
             for bear_glob in default_actions:
                 if fnmatch(result.origin, bear_glob.replace('*', '**')
-                           if bear_glob=='*' else bear_glob):
+                           if bear_glob == '*' else bear_glob):
                     action = default_actions[bear_glob]
                     break
             else:

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob=='*' else bear_glob):
Copy link
Collaborator

Choose a reason for hiding this comment

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

E225 missing whitespace around operator

Origin: PycodestyleBear (E225), Section: autopep8.

@@ -138,7 +138,8 @@ def autoapply_actions(results,
action = default_actions[result.origin]
except KeyError:
for bear_glob in default_actions:
if fnmatch(result.origin, bear_glob):
if fnmatch(result.origin, bear_glob.replace('*', '**')
if bear_glob == '*' else bear_glob):
Copy link
Contributor

Choose a reason for hiding this comment

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

Sudnt this be if not "**" in bear_globs then replace '*' with '**' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@newbazz wip. Please review only after I ping you😉

Copy link
Contributor Author

@ishanSrt ishanSrt Jan 29, 2018

Choose a reason for hiding this comment

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

actually the solution is not simple as that, I have to check for each * I find whether the next is also not a * and only then I need to convert that. I am trying to find an elegant one liner for that. Also need to write a test over here.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is what i am telling bro, if ** not in x and '*' in x then replace but then there can be something like a*b**c right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes and I don't want to write an ugly loop

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah exactly!! :)

Copy link
Member

Choose a reason for hiding this comment

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

You could also use re.subs(r'\*[^*]', '**', text)

Copy link
Member

Choose a reason for hiding this comment

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

Ah maybe the regex has to be expanded to

\*([^*]|$)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Makman2 that is still gonna match the last * in any number of consecutive *s

Some bears' ``result.origin`` have ``os.sep`` in them which makes
the glob ``*`` not match against them. This commit removes the
implementation of this glob altogether while comparing globs with
bear names and converts it into ``**``.

Closes coala#5041
Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

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

Needs tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

6 participants