Refactor the generator scripts for unicode tables #1623
Conversation
tools/unicode_c_source.py
Outdated
| self.__data = "" | ||
|
|
||
| def complete_header(self, completion): | ||
| header = self.__header[:len(self.__header) - 3] |
There was a problem hiding this comment.
If we are removing the last three characters always, why don't we leave out from the string in the first place? Or even better do we need to remove those characters at all? can't we just append the completion as a comment and leave the license string alone?
tools/unicode_c_source.py
Outdated
| def complete_header(self, completion): | ||
| header = self.__header[:len(self.__header) - 3] | ||
| header += " *\n" + completion | ||
| header += " */\n\n" |
There was a problem hiding this comment.
Personally I'm not really fond of adding strings together. I would use a list add any required lines then join them together with \n-s
tools/unicode_c_source.py
Outdated
| def add_table(self, table, table_name, table_type, table_descr): | ||
| self.__data += (table_descr + "\n" | ||
| "static const " + table_type + " jerry_" + table_name + "[] JERRY_CONST_DATA =\n" | ||
| "{\n" + format_code(table, 1) + "\n};\n\n") |
There was a problem hiding this comment.
If the __data is a list then this could look like this:
self.__data.append(table_descr)
self.__data.append("static const %s jerry_%s[] JERRY_CONST_DATA" % (table_type, table_name))
self.__data.append("{")
self.__data.append(format_code(table, 1))
self.__data.append("}")
self.__data.append("") # for an extra empty line
then when writing out the data we can just simple join them together. What do you think?
There was a problem hiding this comment.
@galpeter : I like it, thanks! It's a prettier solution and I've changed these parts based on your suggestions.
robertsipka
force-pushed
the
refactor_unicode_scripts
branch
from
9c41d0f to
38f0e06
Compare
tools/unicode_c_source.py
Outdated
| class Source(object): | ||
| def __init__(self, filepath): | ||
| self.__filepath = filepath | ||
| self.__header = [] |
There was a problem hiding this comment.
I you are creating the list here, why not put the license and the empty line in it by default?
tools/unicode_case_conversion.py
Outdated
| spec_casing_file = os.path.basename(script_args.special_casing) | ||
|
|
||
| header_completion = [] | ||
| header_completion.append("/* This file is automatically generated by the %s script" % os.path.basename(__file__)) |
There was a problem hiding this comment.
No need to call append, directly put the strings into the list above.
tools/unicode_case_conversion.py
Outdated
| conv_length = lengths[idx] | ||
| # Remove all founded case mapping from the conversion tables after the scanning method | ||
| idx = 0 | ||
| while idx != len(start_points): |
There was a problem hiding this comment.
hmmm... what if we do something like this:
for idx in range(len(start_points)):
...
?
robertsipka
force-pushed
the
refactor_unicode_scripts
branch
from
38f0e06 to
30e4d05
Compare
|
@galpeter : thanks for your suggestions, I've updated the patch. |
tools/unicode_c_source.py
Outdated
| self.__data.append("") # for an extra empty line | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Why so many empty lines? Just have one empty line between methods inside a class
tools/unicode_case_conversion.py
Outdated
| try: | ||
| result += unichr(int(unicode_char, 16)) | ||
| except NameError: | ||
| result += chr(int(unicode_char, 16)) |
There was a problem hiding this comment.
I'm not sure about this, but: maybe we should extract the int call before the try-catch. What do you think?
There was a problem hiding this comment.
I've updated it accordingly.
robertsipka
force-pushed
the
refactor_unicode_scripts
branch
from
30e4d05 to
60c53c2
Compare
Extract the source code generator methods into a separated `unicode_c_source.py` script. Fix the generator scripts to make them compatible with both Python2 and Python3. Remove pylint warnings. JerryScript-DCO-1.0-Signed-off-by: Robert Sipka rsipka.uszeged@partner.samsung.com
robertsipka
force-pushed
the
refactor_unicode_scripts
branch
from
60c53c2 to
5ef1f3a
Compare
|
lgtm |
4 participants
Extract the source code generator methods into a separated
unicode_c_source.pyscript.Fix the generator scripts to make them compatible with both Python2 and Python3.
Remove pylint warnings.