Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(config): config compiler plugins for auto-patching and supporting legacy import_preset syntax #150

Merged
merged 7 commits into from
Sep 17, 2017

Conversation

lotem
Copy link
Member

@lotem lotem commented Sep 15, 2017

Fixes #149 , also:

  • Fixed two bugs in config dependency resolution append/merge algorithm.
  • Disabled the old Customizer.

TODO:

  • Remove the patched copy on next upgrade.
  • Schema copied to user direction without notification, as well as symlinks should be removed too,
  • after applying the fallback resource resolver to dictionary files.

@lotem lotem requested review from osfans and Prcuvu September 15, 2017 15:02
@lotem lotem changed the title feat(config): config compiler plugins for auto-patching *.custom.yaml files and support old import_preset: syntax feat(config): config compiler plugins for auto-patching and supporting legacy import_preset syntax Sep 15, 2017
Copy link
Contributor

@kionz kionz left a comment

Choose a reason for hiding this comment

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

/ku:l/

@kionz kionz merged commit 68479eb into master Sep 17, 2017
@osfans
Copy link
Contributor

osfans commented Sep 18, 2017

android ndk 有編譯錯誤,不知有沒有解法。
linux的gcc和clang編譯通過。

  In file included from /home/osfans/trime/app/src/main/jni/librime/src/rime/config/auto_patch_config_plugin.cc:6:
  /home/osfans/trime/app/src/main/jni/librime/src/rime/config/config_compiler_impl.h:37:33: error: member reference base type 'char const[12]' is not a structure or union
    return stream << representable.repr();
                     ~~~~~~~~~~~~~^~~~~
  /home/osfans/trime/app/src/main/jni/librime/src/rime/config/auto_patch_config_plugin.cc:28:13: note: in instantiation of function template specialization 'rime::operator<<<std::__ndk1::basic_stringstream<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >, char [12]>' requested here
    LOG(INFO) << "auto-patch " << resource->resource_id << ":/__patch: "

@Prcuvu
Copy link
Contributor

Prcuvu commented Sep 18, 2017

@osfans ndk是怎么识别成那个对RepresentableT的左移操作符的?难道ndk没有std::ostreamconst char *的左移操作符?

@lotem
Copy link
Member Author

lotem commented Sep 18, 2017

這代碼是我寫的嘛?我瞧瞧。

喔。原來是 ReferenceDependency 兩種類型,我給他總結成模板參數了。
沒法唔,我太喜歡用模板了。編譯器展開以後應該還是他倆。
妥了,我就多寫幾句,還給他寫成兩樣吧。

@lotem lotem self-assigned this Sep 18, 2017
@lotem
Copy link
Member Author

lotem commented Sep 18, 2017

@osfans Pushed 71817a0

@Prcuvu Prcuvu deleted the config branch September 19, 2017 00:48
@osfans
Copy link
Contributor

osfans commented Sep 19, 2017

@lotem 果然編譯過了。是因爲ndk檢查嚴格嗎?

@osfans
Copy link
Contributor

osfans commented Sep 19, 2017

發現一個問題,不能__include自己
錯誤如下:

E0919 11:44:33.235180 29953 config_compiler.cc:504] unresolved dependency: Include(luna_pinyin.schema:/test/punctuator)
E0919 11:44:33.235309 29953 config_compiler.cc:504] unresolved dependency: PendingChild(luna_pinyin.schema:/punctuator)
E0919 11:44:33.235345 29953 config_component.cc:213] error loading config from: luna_pinyin.schema

luna_pinyin.schema.yaml

test:
  punctuator:
    full_shape:
      ' ' : { commit: ' ' }
      ',' : { commit: , }

punctuator:
  __include: /test/punctuator

@lotem
Copy link
Member Author

lotem commented Sep 19, 2017

@osfans 因爲有個看不見的 auto-patch。

__patch: luna_pinyin.custom.yaml:/patch?

test:
  punctuator:
    full_shape:
      ' ' : { commit: ' ' }
      ',' : { commit: , }

punctuator:
  __include: /test/punctuator

這樣看的話,的確有衝突:punctuator/__include 要用到尚待 patch 的節點 test/punctuator——在應用 __patch 之前,無法確定要引用內容(的最終版本)。

怎麼解決呢?
一個思路是,讀完YAML只要任何子節點上有操作,就不自動打補丁啦。這也就是說,只給未使用新語法的文件自動打補丁。
另一個思路是,先對當前文件未解決的依賴求值,再打補丁。這樣呢,導致自動補丁在語義上有所不同——補丁可能修改被引用過的節點(test),那麼先期求值的引用會保持舊的值。

無論如何,如果自己寫等效的補丁,還是會發生衝突的。——或者不應當認爲這種用法有錯?

@osfans
Copy link
Contributor

osfans commented Sep 20, 2017

第一個思路的話,要不就判斷luna_pinyin.custom.yaml是否存在?不存在就不打,應該能滿足依賴關係。
但如果存在的話,還是會有錯誤。

不知怎麼解決好了。

@kionz
Copy link
Contributor

kionz commented Sep 20, 2017

@osfans @lotem i have a fix #154
allowing references to access unresolved node

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ConfigCompiler 插件
5 participants