FooSoft / yomichan-anki

Plugin for sentence/vocab mining Japanese books in Anki.
https://foosoft.net/projects/yomichan-anki
Other
58 stars 12 forks source link

Context sentence properly cut off at newline #6

Closed felixvd closed 9 years ago

felixvd commented 9 years ago

The newline character is taken into account 20 lines earlier when looking for the start of the segment, but not when looking for its end. This fixes behavior when lines don’t end in punctuation (e. g. notes taken in lectures), where it used to result in excessively long {sentence} fields.

FooSoft commented 9 years ago

I see what you mean, but the issue is that the feature extracts sentences, not lines. This is important when dealing with documents that are hard-wrapped to a certain column count. With your change, I think you will get a lot of broken sentences : )

Also, as a minor nitpick, in Python you don't need the extra parens : )

if a or b instead of if (a or b)

I'm a C++ programmer so I used to do the same thing all the time

felixvd commented 9 years ago

Mmmh, hard-delimited text files are a good argument. Maybe it's worth a check box? It's definitely convenient for taking notes at least.

Thanks for the heads up, too :)

On 11 February 2015 at 17:59, Alex Yatskov notifications@github.com wrote:

I see what you mean, but the issue is that the feature extracts sentences, not lines. This is important when dealing with documents that are hard-wrapped to a certain column count. With your change, I think you will get a lot of broken sentences : )

Also, as a minor nitpick, in Python you don't need the extra parens : )

if a or b instead of if (a or b)

I'm a C++ programmer so I used to do the same thing all the time

Reply to this email directly or view it on GitHub https://github.com/FooSoft/yomichan/pull/6#issuecomment-73850390.

FooSoft commented 9 years ago

Maybe what you want to do is add a parameter for the current line or something, so that you can use that instead of sentence. Or maybe just an option, as you said.

felixvd commented 9 years ago

A {line} parameter might not be a bad idea. Although I thought it would be unintuitive at first, it might make sense when taking notes. I'll close this request because I pushed the other feature into the same branch. I dumb.