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

Check: all pref keys in preference.xhtml should be init in prefs.js #54

Open
northword opened this issue Oct 1, 2024 · 11 comments · May be fixed by #65
Open

Check: all pref keys in preference.xhtml should be init in prefs.js #54

northword opened this issue Oct 1, 2024 · 11 comments · May be fixed by #65
Labels
enhancement New feature or request

Comments

@northword
Copy link
Owner

No description provided.

@northword northword added the enhancement New feature or request label Oct 1, 2024
@volatile-static
Copy link
Contributor

要我说不如像Chartero一样统一到package.json里

@northword
Copy link
Owner Author

这也解决不了这个 issue 的问题的嘛,这个 issue 的问题通常出现在 重构插件后,变更了一些 pref key,比如 northword/zotero-format-metadata@d6db4d2#diff-c4fc1c0947464c11133cca736e3f65a4e99567a38d9fab884e046bdd1b260839 这种,只在 prefs.js 里改了,但把 preference.xhtml 里的拉下了。

我觉得这个最好能是在 build 之后,打包之前去检查,

甚至可以把 ftl 检查那个一起抽出来,做成 Addon Linter 类似的东西(饼)

@volatile-static
Copy link
Contributor

我再稍微解释一下我的插件结构

  1. 首先,没有 prefs.js 文件,编译时会从 package.json 中生成
  2. 其次,在 preference.xhtml 里可以像常用的__addonName__一样自动替换,标签会简洁很多
  3. 此外,TS代码中能实现严格的类型提示,因为 package.json 中给出了默认值

我理解你的问题应该是想加个防呆措施,但如果都能统一在一处,那自然就无呆可防了

@northword
Copy link
Owner Author

是的,就是防呆。

有这么几个问题:

  • 这种方案,当 __enableRealTimeDashboard__ 没有在 package.json 中定义,但在 preference.xhtml 中使用了的时候,首选项窗格会直接打不开,需要手动去 build/ 里自己找是哪儿的错。理想中是在终端直接打印哪个 key 不存在,就像现在的 ftl message 一样。
  • 这样会不会有些抽象,和原有的方式已经差很多了,新手会不会理解不了()
  • 如果这样搞,Scaffold 的配置怎么设计呢?(我不太期望都塞进 package.json 里的,因为
    • 毕竟 scaffold 已经有自己的配置文件了,
    • 但不放进 package.json,又不太好在 ts 里实现提示,因为在插件里从作为开发依赖项的 scaffold 的配置文件里导入东西总显得奇奇怪怪的

@volatile-static
Copy link
Contributor

  1. 是的,感谢提醒,我再改进一下
  2. 确实抽象,但是一劳永逸的,不太需要彻底理解
  3. 我感觉更适合放在模板,不过要放在脚手架应该也行,可能就在plugin-config.ts里配置?

@windingwind
Copy link
Collaborator

我倾向于尽可能减少对编译后插件结构的改变,也就是在可以直接使用原文件的情况下尽可能不去完全生成新文件,主要是方便新手理解

看讨论以后感觉也许最合适的还是做一个针对prefs的检查脚本,类似locale检查的脚本,每次编译触发

另外关于放package.json,我感觉大趋势是配置文件独立出来。但是独立出来的话,和单独的prefs.js也没大区别了

@windingwind
Copy link
Collaborator

如果确实需要类型提示,一种可行的办法是每次编译从prefs.js编译出一个d.ts,我觉得比放在package.json的野路子要好一些,对新手的学习压力也小(真的不需要可以关掉)

@volatile-static
Copy link
Contributor

都可以,模板确实需要考虑一些学习成本。不过对你俩来说,还是建议统一省事一些。需要的话我这两天有空可以pr

@northword
Copy link
Owner Author

我也倾向于做成和现有 locale 检查类似的模式。

如果确实需要类型提示,一种可行的办法是每次编译从prefs.js编译出一个d.ts

prefs.js 里的 key 是带 __prefPrefix__ 的,但在 js/ts 代码里用 getPref() 时候 key 是没有这个前缀的,并且每个人也许可以设置不同的前缀(和前缀占位符),怎么处理这个呢?

模板确实需要考虑一些学习成本

也许可以在 preference.xhtml 里也保留基本的结构,比如不是写

<checkbox native="true" data-l10n-id="enableRealTimeDashboard" __enableRealTimeDashboard__ />

而是

<checkbox native="true" data-l10n-id="enableRealTimeDashboard" preference="enableRealTimeDashboard" />

因为 preference 这个属性是有意义的,可以不省略。

这样下来,就只是增加了一个 Config.build.pref.prefixPrefix 配置,

  • 要求 xhtml 和 prefs.js 里都写不加前缀的 prefKey,由 scaffold 去增加前缀
    • 是否可以配置这个前缀或默认 extensions.zotero.{namespace}
    • 当已有前缀的(以 extensions 开头),跳过加前缀
    • 这一步放在 replace 之后
  • 然后检查 xhtml 里的 prefKey 是否都在 prefs.js 里给出默认值。

@volatile-static
Copy link
Contributor

这个想法不错,普适性比较强

是否可以配置这个前缀或默认 extensions.zotero.{namespace}

最好是默认extensions.zotero.addonName吧,实在特殊的需求可以代码里特殊处理

@northword
Copy link
Owner Author

最好是默认extensions.zotero.addonName吧

配置里是有 namespace 的,

/**
* namespace of plugin.
*
* This attribute is also used to prevent plugin conflicts,
* it may be used for plugin HTML element ids, preference prefixes,
* fluent filename prefixes, fluent message prefixes, etc.
*
* Unlike the plugin id, this value should be HTML ID and Fluent Message compliant,
* i.e.: contain at least one character and only letters and dashes.
*
* 插件命名空间。
*
* 这个属性也用于防止插件冲突,它可能被用于插件的 HTML 元素 id,
* 首选项前缀,Fluent 文件名前缀、Fluent message 前缀等。
*
* 与插件 ID 不同的是,这个值应是符合 HTML ID 和 Fluent Message 规范的,
* 即:至少包含一个字符,仅包含字母和短横线。
*
* @default kebabCase(name)
*/
namespace: string;
,默认是 addonName 全小写,空格换为 -

addonName 可能包含空格,处理下来可能和 namespace 是差不多的。

这个目前用在了 ftl 的前缀,可以在这里也用起来。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants