Skip to content

Feat: Accessbilities to Dictionary & UserDictionary #80

Merged
hchunhui merged 27 commits intohchunhui:masterfrom
TsinamLeung:master
Apr 17, 2021
Merged

Feat: Accessbilities to Dictionary & UserDictionary #80
hchunhui merged 27 commits intohchunhui:masterfrom
TsinamLeung:master

Conversation

@TsinamLeung
Copy link
Copy Markdown
Contributor

#77
Features that can access Dictionary & userdictionary
user could update the userDictionary with specified arguments

Copy link
Copy Markdown
Owner

@hchunhui hchunhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Please see my comments below.

Comment thread src/types.cc Outdated
Comment thread src/lua_gears.cc Outdated
Comment thread src/types.cc Outdated
Comment thread src/types.cc
namespace MemoryReg {
using SyllableId = int32_t; // copy from rime/vocabulary.h

class LuaMemory : public Memory {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with Memory. Can you add a example script to demo how to use it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's my demo script for memory right now;

   mem = Memory(env.ticket)
   mem:dictLookup("onf",false)
   while (mem:dictResultExhausted() != false)
   do
       dict_entry = mem:dictPeek()
       print(dict_entry.text)
       print(dict_entry.weight)
       print(dict_entry.preedit)
       print(dict_entry.comment)
       ...
   end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with Memory. Can you add a example script to demo how to use it?

I'm about to write a doc about librime-Lua, and I will add the usage of Memory and other components to the doc

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we wrap the dict class directly instead of the Memory class? In the example, the Memory object is only used to iterator a dict. I still want to know what Memory is.

Copy link
Copy Markdown
Contributor Author

@TsinamLeung TsinamLeung Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Memory class is a set of dictionary and UserDictionary for this schema, but there's more.
Memory has a method named memorize it is a callback of commit() and CommitEntry which contains Histories of committed DictEntry would be passed to it. CommitEntry would help to record commits to UserDictionary.
I discovered it recently, so I am trying to write a Lua callback and pass commitEntry to the lua right now.
It might help lua users to handle Userdictionary well.

Copy link
Copy Markdown
Contributor Author

@TsinamLeung TsinamLeung Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory mostly did a lot of job handling UserDictionary for Userdictionary recording needs time tick which is handled by methods like StartSession FinishSession or receiving deleted entry from FrontEnd or stops recording entry when facing unhandled key.
I think If users want to build a completed translator, some methods of handling Userdictionary is necessary.

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

TsinamLeung commented Nov 21, 2020

My target is to build a competed translator via Lua.
Memory could handle commit history after commit by user and dictionaries.
But Access to Dict and UserDict just the first step, I think there are further steps for a qualified translator.

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

Accessing other schema's dictionary required a custom schema, so I add a new function on SchemaReg

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

documents are reday now

@hchunhui
Copy link
Copy Markdown
Owner

Looks good! I will try building and running the code.

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

TsinamLeung commented Nov 26, 2020

Looks good! I will try building and running the code.

thanks!

BTW, could you check out this document, any improvement?
documents

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

Just add a example of Memory.

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

I found there's a problem that Lua using simpleCandidate which would probably disable memorize callback memory.

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

The new following commit implemented a ways of associate phrase!
by building a pure vocabulary dictionary and do query by lua!

@TsinamLeung TsinamLeung reopened this Dec 28, 2020
@hchunhui
Copy link
Copy Markdown
Owner

@ZenamLeong Sorry for the late reply. Today I have tried and reviewed your code, and I have added some commits (fix some memory bugs, rewrite some code, etc.). Please see the commit logs. Feel free to revert or fix them if I'm wrong.

I'm willing to merge the code, but there are still some issues:

  • Some interfaces are inconsistent. e.g. there is a limit argument in dict_lookup(), but there is no in userlookup().
  • associate_component.lua is broken. It should be fixed.
  • It would be better to extract the dictionary from LuaMemory (this issue can be done in the future PRs).

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

  • Thx for your commit! I will check it out later!

  • associate_component.lua needs a extra dictionary i would add it later, It shows a method to associating vocabulary using dictionary.

  • It is ok to extract dictionary out of memory And i'll try to wrap in further Commits.

@hchunhui
Copy link
Copy Markdown
Owner

@TsinamLeung Thanks for your contribution!

My plan is to merge the pull request next week, to allow other works based on it. If you have no further fixes, I will delete associate_component.lua before merging. You can reimplement it in the future.

@TsinamLeung
Copy link
Copy Markdown
Contributor Author

@TsinamLeung Thanks for your contribution!

My plan is to merge the pull request next week, to allow other works based on it. If you have no further fixes, I will delete associate_component.lua before merging. You can reimplement it in the future.

ok thank you for reply,
sorry about that I'm busy recent month, so I probably can't commit anything before July.
Sorry for the delay.
thank you again!

@hchunhui hchunhui merged commit 15bd231 into hchunhui:master Apr 17, 2021
shewer added a commit to shewer/librime-lua that referenced this pull request Apr 18, 2021
Signed-off-by: shewer <shewer@gmail.com>
shewer added a commit to shewer/librime-lua that referenced this pull request May 2, 2021
Signed-off-by: Shewer Lu <shewer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants