Add additional info to dictionary entries and use standard arg parsing #18
Conversation
Cleanup Update README Fix bug in JMnedict parsing Update Makefile Fix Bug in minifying index Fix Bug in minifying index Fix Bug in minifying index Update README Rename
mymro
changed the title
|
@jrfonseca When will you merge the commits? |
| @@ -20,9 +20,6 @@ then processed with ImageMagick's | |||
| `mogrify -colorspace gray -level 0%,111.11% -define PNG:compression-level=9` | |||
| to look like E-Ink display. | |||
| --> | |||
|
|
|||
|  | |||
|
|
|||
There was a problem hiding this comment.
I'd like to keep an example of inflection.
| @@ -237,14 +241,14 @@ def write_index(entries, dictionary_name, title, stream, respect_re_restr=True, | |||
| entry.sentences.sort(reverse=True, key = lambda sentence: sentence.good_sentence) | |||
| for sentence in entry.sentences: | |||
| stream.write(' <div class="sen">\n') | |||
| stream.write(' <span>' + sentence.japanese + '</span>\n') | |||
| stream.write(f" <span>{sentence.japanese}</span>\n") | |||
There was a problem hiding this comment.
f-strings is great, but for future reference, I'd prefer if we standardized on single quotes ' strings, as this makes it easier to emit " in the generated XML/HTML
dictionary.py
Outdated
| stream.write('<?xml version="1.0" encoding="utf-8"?>\n') | ||
| stream.write('<package unique-identifier="uid">\n') | ||
| stream.write(' <metadata>\n') | ||
| stream.write(' <dc-metadata xmlns:dc="http://purl.org/metadata/dublin_core">\n') | ||
| stream.write(' <dc:Identifier id="uid">%s</dc:Identifier>\n' %(hex(hash(title)).split('x')[1])) | ||
| stream.write(' <dc:Title><h2>%s</h2></dc:Title>\n' %title) | ||
| stream.write(f" <dc:Identifier id=\"uid\">{hex(hash(title)).split('x')[1]}</dc:Identifier>\n") |
There was a problem hiding this comment.
For example, here, using f'...' would avoid escaping the single quote.
jmdict.py
Outdated
|
|
||
| arg_parser = argparse.ArgumentParser(description='Build the JMdict or JMnedict for Kindle', formatter_class=argparse.RawTextHelpFormatter) | ||
| arg_parser.add_argument('-s', '--sentences' | ||
| ,action='store' |
There was a problem hiding this comment.
I don't want to nitpick, but commas like this look awkward to me.
I'm starting to think it might be better to start using some sort of automated Python code formatting tool to ensure code is consistent.
There was a problem hiding this comment.
@jrfonseca How would you prefer them?
There was a problem hiding this comment.
Trailing commas. https://www.python.org/dev/peps/pep-0008/#when-to-use-trailing-commas
Honestly, I think the easiest thing to do is to use an automated tool to format like https://www.python.org/dev/peps/pep-0008/ . I know there are a few, and I even recall to use one a couple of years ago, but I forgot which.
|
@jrfonseca I will probably update this commit over the weekend |
Fix auto format in action Fix auto format in action Fix auto format in action Fix auto format in action a a Change Actions for pull_request_target a a a
mymro
force-pushed
the
master
branch
2 times, most recently
from
6320a8e
to
ad25d56
Compare
|
@jrfonseca The code will now be checked for being correctly formatted with black in each push or pull request. In the README there is a section that explains how to automatically let black format the source files. I have also included the screenshot with an inflection and put the images on two lines in order for them not to take up so much space. As soon as this pull request is merged, I will check the whole repository for compliance with black and then push eventual changes. |
|
@jrfonseca Could you please merge the pull request. |
|
Thanks. |
This pull request resolves #14 and uses the standard arg parser.