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

Fix resx in other dll cant be read by SourceGenerators (#4) #5

Closed
wants to merge 1 commit into from

Conversation

emako
Copy link

@emako emako commented Nov 30, 2023

No description provided.

@emako
Copy link
Author

emako commented Nov 30, 2023

// Example code to redirect the Assembly
I18NAssemblyProvider.SetAssembly<LangKeys>();

@feast107
Copy link
Member

feast107 commented Dec 1, 2023

首先,非常感谢你提供的修改。但我觉得这个方式依然有改进的空间:

  • 需要主动调用SetAssembly()是个危险的行为,这让LangKeys的创建与启用分布在了不同的地方,让一个可能引发的问题依然保持了其隐秘性。🤔
  • Compiled Binding support for Avalonia #2 (comment) 中提到了针对Avalonia平台的优化,通过更多的生成来支持编译绑定,减少反射的成分。我十分支持这一建议并已经在思考如何不使用反射来完成这一任务,这也就意味着Provider将会以另一种形式呈现。就目前来看使用GetExecutingAssembly()也许更好一些。🥰
  • 在代码格式上对垂直对齐有一些要求,尽量减少一些不必要的、仅因为格式化而产生的code review。

@emako
Copy link
Author

emako commented Dec 2, 2023

一开始也是打算用GetExecutingAssembly,但这就意味着需要在拥有resx的dll里主动调用一次I18NExtension里的方法,让static构造跑起来,比如_ = I18NExtension.Translate(...),这会让代码可读性极差
最好还是得重构一下I18NExtension的初始化,SetAssembly作为当前版本下的参考方式。

GetExecutingAssembly的是Antelcat.I18N,这就找不到SourceGen的用户Assembly了。

@feast107
Copy link
Member

feast107 commented Dec 2, 2023

这个问题已经在:caa9fa1 中使用 [ModuleInitializer] 得到修复,通过模块的初始化器来调用Provider,但不幸的是他是System.Runtime.CompilerService.ModuleInitializer,这意味着他依然需要依赖反射

@emako
Copy link
Author

emako commented Dec 2, 2023

体验一下

@feast107
Copy link
Member

feast107 commented Dec 2, 2023

Nuget 包已经移动到了 Antelcat.I18N.WPF 并且将生成器抽离成了独立的模块。没有API的变动 🥰

@emako
Copy link
Author

emako commented Dec 2, 2023

终于有new nuget包了,Хорошо

@emako emako closed this Dec 2, 2023
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