-
Notifications
You must be signed in to change notification settings - Fork 46
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
base_parser: support relative path include files #399
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #399 +/- ##
==========================================
+ Coverage 81.19% 81.30% +0.11%
==========================================
Files 57 57
Lines 7358 7383 +25
==========================================
+ Hits 5974 6003 +29
+ Misses 1384 1380 -4 ☔ View full report in Codecov by Sentry. |
9e33b51
to
e42a6ff
Compare
This commit adds support for including multiple relative paths from the same directory by storing a stack of all included files that have not finished being parsed, and the number of lines that still need to be parsed in each file. Previously, only includes of nested relative paths were supported as the currently parsed file was not reverted to the previous file when the parsing of an included file was completed. This led to further includes needing the be relative to the latest included file.
…etNextLine() recursively
e42a6ff
to
d15392b
Compare
@@ -42,6 +42,8 @@ def __init__(self, log="BaseParser"): | |||
self.PPs = [] | |||
self._Edk2PathUtil = None | |||
self.TargetFilePath = None # the abs path of the target file | |||
self.FilePathStack = [] # a stack containing a list of size 2: [filepath, line_count] |
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.
NIT:
Since you're likely dealing with a large set of data, the following solution probably doesn't make a lot of sense performance wise but if you wanted to improve readability of the code you could do something like a namedtuple.
However the object instantiation might be too much for a large dataset
from collections import namedtuple
# Define the namedtuple type
FilePathRecord = namedtuple('FilePathRecord', ['filepath', 'line_count'])
self.FilePathStack = [] # a stack containing FilePathRecord instances
self.FilePathStack.append(FilePathRecord('my_file_path', 100))
self.FilePathStack[0].file_path
return False | ||
line_count = self.FilePathStack[-1][1] | ||
if line_count - 1 > 0: | ||
self.FilePathStack[-1][1] -= 1 |
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.
NIT:
updated_line_count = self.FilePathStack[-1][1] - 1
if updated_line_count > 0:
self.FilePathStack[-1][1] -= updated_line_count
@@ -92,10 +92,10 @@ def __ParseLine(self, Line, file_name=None, lineno=None): | |||
if sp is None: | |||
raise FileNotFoundError(include_file) | |||
self.Logger.debug("Opening Include File %s" % sp) | |||
self._PushTargetFile(sp) | |||
lf = open(sp, "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.
NIT: I know you didn't write this but if we switch to with we can drop lf.close()
This commit adds support for including multiple relative paths from the same directory by storing a stack of all included files that have not finished being parsed, and the number of lines that still need to be parsed in each file.
Previously, only includes of nested relative paths were supported as the currently parsed file was not reverted to the previous file when the parsing of an included file was completed. This led to further includes needing the be relative to the latest included file.
closes #394