-
Notifications
You must be signed in to change notification settings - Fork 913
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
fix paths (and regex for paths) comparison issues #1592
Conversation
…ndows path delimiter for regex comparison
@@ -116,6 +116,8 @@ def test_loggers(self): | |||
base, ext = os.path.splitext(this_file) | |||
if ext == '.pyc': | |||
this_file = base + '.py' | |||
if os.path.sep == '\\': | |||
this_file = this_file.replace(os.path.sep, r'\\') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make more sense to re.escape(this_file)
?
Same below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for sharing this with us! will change to use re.escape
@@ -116,6 +116,7 @@ def test_loggers(self): | |||
base, ext = os.path.splitext(this_file) | |||
if ext == '.pyc': | |||
this_file = base + '.py' | |||
this_file = re.escape(this_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please apply the escaping where it is being used - in this case directly in line 182.
@@ -92,6 +93,7 @@ def test_rosconsole__logging_format(): | |||
base, ext = os.path.splitext(this_file) | |||
if ext == '.pyc': | |||
this_file = base + '.py' | |||
this_file = re.escape(this_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please apply the escaping where it is being used - in this case directly in line 114.
Thank you for the patch and for iterating on it. |
* add os.path.normcase for return value of Logger.FindCaller, escape windows path delimiter for regex comparison * escape os path separator for regular expression comparison * use os.path.sep to indicate path separator * use re.escape to escape metacharacters * apply escaping only when the string is used for comparison
\
as path separator (Windows, Mac OS), need to escapeos.path.sep
for regular expression comparison