northword / zotero-plugin-scaffold

A scaffold for Zotero plugin development
https://www.npmjs.com/package/zotero-plugin-scaffold
GNU Affero General Public License v3.0
8 stars 5 forks source link

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

Open northword opened 1 month ago

volatile-static commented 1 month ago

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

northword commented 1 month ago

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

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

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

volatile-static commented 1 month ago

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

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

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

northword commented 1 month ago

是的,就是防呆。

有这么几个问题:

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

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

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

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

windingwind commented 1 month ago

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

volatile-static commented 1 month ago

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

northword commented 1 month ago

我也倾向于做成和现有 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 配置,

volatile-static commented 1 month ago

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

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

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

northword commented 1 month ago

最好是默认extensions.zotero.addonName吧

配置里是有 namespace 的,https://github.com/northword/zotero-plugin-scaffold/blob/56210f961c49ea1a9583a9a346e278af31223bd7/src/types/config.ts#L55-L75 ,默认是 addonName 全小写,空格换为 -

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

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