Skip to content

modifications to dart.h and vterbi.cpp files - #4

Closed
frawamudi wants to merge 6 commits into
fasiha:masterfrom
frawamudi:patch-2
Closed

modifications to dart.h and vterbi.cpp files#4
frawamudi wants to merge 6 commits into
fasiha:masterfrom
frawamudi:patch-2

Conversation

@frawamudi

@frawamudi frawamudi commented Jan 16, 2023

Copy link
Copy Markdown

The C++ storage type specifier "Register" was removed to enable build for C++17 also the edit files were clang formatted

@fasiha

fasiha commented Jan 17, 2023

Copy link
Copy Markdown
Owner

Thanks for this! I see this unmerged PR in the upstream MeCab repo from a year ago taku910/mecab#66 so I'm happy to accept the fix to get this to compile as C++17.

In order to keep the differences with the upstream/original C++ code as minimal as possible though, can I beg you to revert your clang-format changes?

Let me know, if you're busy I can try to undo the whitespace changes, keeping just the register changes and maybe the new test case (I can't easily see the test in the diff).

Again, thank you!

@frawamudi

Copy link
Copy Markdown
Author

Aiite will revert.

@frawamudi

Copy link
Copy Markdown
Author

Hi will work on this. been very busy with work

@frawamudi

Copy link
Copy Markdown
Author

I just fixed the clangtidy formatted files

@fasiha

fasiha commented Aug 23, 2023

Copy link
Copy Markdown
Owner

Thanks! Could you help me understand the rewrite of src/viterbi.cpp? It looks like calc_alpha and Viterbi::forwardbackward and several other functions were removed, were these unused…?

Also, I see that your taku910/mecab#66 has a really small diff, do you think I could just merge that patch into this repo?

@frawamudi

Copy link
Copy Markdown
Author

Thanks! Could you help me understand the rewrite of src/viterbi.cpp? It looks like calc_alpha and Viterbi::forwardbackward and several other functions were removed, were these unused…?

Also, I see that your taku910/mecab#66 has a really small diff, do you think I could just merge that patch into this repo?

oh so sorry about that, its a mix up from my end. will resolve that asap

@frawamudi frawamudi closed this by deleting the head repository Sep 22, 2025
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