Session-only New Spellings

jxmorris12 / language_tool_python

a free python grammar checker 📝✅

GNU General Public License v3.0

434 stars 64 forks source link

Session-only New Spellings #35

Closed aokellermann closed 3 years ago

aokellermann commented 3 years ago

Changes

Implements optional feature that enables new spellings to not persist past the current server session. This makes it such that the global spellings file before server start and after server exit are identical.
Adds unit test.
Removes unreliable __del__ function (that is never actually called in my testing) in favor of context manager.

jxmorris12 commented 3 years ago

Hi @aokellermann -- thanks! I think these are worthwhile changes.

I'm unsure about the name new_spellings_only_current_session, and the fact that it's a member variable intended to be set outside of the class. Why not pass it into the constructor? I'd also like to suggest the name new_spellings_persist (and have it mean the opposite of current meaning), though I'm open to ideas.
While useful, _unregister_spellings seems like a difficult bit of code to write and could be error prone. Do you mind adding a test for this method?

aokellermann commented 3 years ago

@jxmorris12

I agree -- I think new_spellings_persist is a better name.

I already added a test case in test_major_functionality.py that compares file hashes before registering and after unregistering. You can see it fail here. The function is fairly ugly, but fast. I couldn't think of a better way to do it.

jxmorris12 commented 3 years ago

@jxmorris12

I agree -- I think new_spellings_persist is a better name.

I already added a test case in test_major_functionality.py that compares file hashes before registering and after unregistering. You can see it fail here. The function is fairly ugly, but fast. I couldn't think of a better way to do it.

Works for me! Great work, thanks so much