[Tools] Added -d (--detailed) paremeter to unfold 'Misc' contents in memap.py table #2845
Conversation
| argparse_lowercase_hyphen_type, argparse_uppercase_type | ||
|
|
||
| DEBUG = False | ||
| DETAILED = False |
There was a problem hiding this comment.
Global variables should not change, so that if we ever need to run many of these concurrently in the same process, we can. Could we make this an object variable?
There was a problem hiding this comment.
What do you mean by this? just define it as lowercase?
There was a problem hiding this comment.
I mean: Don't use a global variable for something that changes at runtime.
There was a problem hiding this comment.
args cannot be accessed from inside the path_object_to_module_name so, unless I add another parameter to path_object_to_module_name signature and propagate it all the way up to the main, I don't see how to implement this without a global variable.
Is this what you would like me to implement? Is there an easier solution?
It is like the 5th time or so that I code something in python so I'd appreciate the input :)
There was a problem hiding this comment.
You could: remove the @statictmethod decorator from path_object_to_module_name giving you access to self (you would also have to add it as the first parameter). Then you could pass detailed into the constructor, with a default value of False, assigning it to an attirbute of the object, say detailed_names. That's how I would do it.
There was a problem hiding this comment.
Thanks for the input :)
Done
| @@ -10,10 +10,11 @@ | |||
| import argparse | |||
| from prettytable import PrettyTable | |||
|
|
|||
| from tools.utils import argparse_filestring_type, \ | |||
There was a problem hiding this comment.
You cannot correctly drop the tools. from the front of this import. Please undo this change, as it has nothing to do with the feature being implemented.
There was a problem hiding this comment.
If I didn't drop the tool from the front of the import I get the following error runing memap.py as standalone from the tools folder (python memap.py ...)
Traceback (most recent call last):
File "memap.py", line 13, in
from tools.utils import argparse_filestring_type,
ImportError: No module named tools.utils
There was a problem hiding this comment.
We'll leave it as it is then.
|
|
||
| # Parse/run command | ||
| if len(sys.argv) <= 1: | ||
| parser.print_help() | ||
| sys.exit(1) | ||
|
|
||
|
|
||
| args = parser.parse_args() | ||
| args, remainder = parser.parse_known_args() |
There was a problem hiding this comment.
We don't use parse_know_args. Could you undo this change as well? it also seems unrelated.
There was a problem hiding this comment.
This one slipped trough (I was porting my changes from an older version of memap.py
I'll change it right away :)
…reno-tridonic-com/mbed-os into feature-detailed-memap # Conflicts: # tools/memap.py
|
retest uvisor |
|
|
||
| # Show Misc unfolded | ||
| if args.detailed: | ||
| memap.detailed_misc = True |
There was a problem hiding this comment.
Could you pass this into the constructor instead?
e.g.
memap = MemapParser(args.detailed)| @@ -40,7 +41,9 @@ class MemapParser(object): | |||
| def __init__(self): | |||
There was a problem hiding this comment.
Then also change this to:
def __init__(self, detailed_misc=False)
""" General initialization
"""
self.detailed_misc = detailed_misc|
|
||
| # Parse/run command | ||
| if len(sys.argv) <= 1: | ||
| parser.print_help() | ||
| sys.exit(1) | ||
|
|
||
|
|
||
| args = parser.parse_args() | ||
| args = parser.parse_args() |
|
@javier-moreno-tridonic-com Can you please address the issues @theotherjimmy pointed? Otherwise looks great! |
screamerbg
changed the title
|
@javier-moreno-tridonic-com I'm going to do the changes myself, so that we can get this into the code base real quick. |
theotherjimmy
force-pushed
the
feature-detailed-memap
branch
from
c717456
to
b0abf60
Compare
theotherjimmy
force-pushed
the
feature-detailed-memap
branch
from
b0abf60
to
2b36d2d
Compare
Description
Adds more detail to the Misc contents from memap.py
Status
READY
Migrations
NO
Todos