search

Dushistov / sdcv

https://dushistov.github.io/sdcv/
GNU General Public License v2.0
294 stars 42 forks source link

KOReader patches #6

Closed Dushistov closed 7 years ago

Dushistov commented 8 years ago

See https://github.com/koreader/sdcv/issues/4 for more information

commit 4900907003c1d2aa0ac6c847c1f6c1db435ea6f1 Author: chrox chrox.huang@gmail.com Date: Thu Mar 12 16:46:21 2015 +0800

Print "save to cache" and "Nothing similar to" messages to stderr so that it won't mess up JSON data

commit b1f5bf15f3219c7b9a8dcccc1f31c763487b4bf5 Author: chrox chrox.huang@gmail.com Date: Wed Oct 29 16:23:00 2014 +0800

Add suppport for HTML and Wikimedia dictionary data

commit fbf9f111e60c22d3b0fccbf10fae4e453a34ca26 Author: chrox chrox.huang@gmail.com Date: Tue Apr 30 17:52:10 2013 +0800 koreader.zip

Add --json-output option

commit 8862e5fc933f7391b300e15f15790678481270de Author: chrox chrox.huang@gmail.com Date: Tue Apr 30 17:50:22 2013 +0800

Add cJSON library

koreader.zip

Dushistov commented 8 years ago

Author of patch is Frenzie

houqp commented 7 years ago

Ping ;) @Dushistov please let us know if any of these patches does not make sense to you. I would love help unfork the project and share the same code base :)

Dushistov commented 7 years ago

@houqp Hi, I'm busy now at my job, and will have more free time at the end of December. Then I deal with bug reports and pull requests.

ecraven commented 7 years ago

@Dushistov if I started PRs for all of the above, is there any that you don't want to merge on principle? (only the json one is of any size, and for that I think I'd prefer to not pull in a json parser just to escape json strings, that can be done better).

Dushistov commented 7 years ago

@ecraven

if I started PRs for all of the above, is there any that you don't want to merge on principle? >(only the json one is of any size, and for that I think I'd prefer to not pull in a json parser just >to escape json strings, that can be done better).

Basic criteria for merging or not is not breaking things and maintainability. From maintainability point of view is including big json library into source tree is bad, but if include it as git submodule and add cmake option to disable it's build, then why not?

Also json support from maintainability point view requires some basic auto tests to verify json output functionality.

Frenzie commented 7 years ago

Btw there's also https://github.com/koreader/sdcv/pull/6 (I admit I don't exactly understand the use case but it's small and harmless.)

Also I've already got 'em as Git commits in my personal fork (from which I generated the diffs before sdcv was on GH) so you can easily cherrypick them.