search

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

config `manifest.json` #31

Closed volatile-static closed 5 months ago

volatile-static commented 5 months ago

要不要把manifest.json整合进plugin-config？

northword commented 5 months ago

最开始的几版是整合了的，但是 i18n 不太好处理，所以就移除了。

https://github.com/northword/zotero-plugin-scaffold/blob/9637457803718d9233ec6531acad097bafe9f949/src/lib/build.ts#L85-L92

现在的是，从 dist 读取 manifest.json，覆盖 id 和 update_url。 https://github.com/northword/zotero-plugin-scaffold/blob/e776abc3742de281df6fcd7825a055417c278a35/src/lib/build.ts#L96-L117

你有什么更好的方案吗？

volatile-static commented 5 months ago

没懂manifest哪里有i18n

northword commented 5 months ago

没懂manifest哪里有i18n

https://zotero-chinese.com/plugin-dev-guide/reference/manifest#manifest-%E7%9A%84%E6%9C%AC%E5%9C%B0%E5%8C%96

当然，最简单的就是在 config 里全部填 __MSG_xxx__，然后其余照旧。但感觉这样一点也不优雅。

windingwind commented 5 months ago

我感觉对原本插件文件结构的保留是必要的，全部拆完了新手更搞不清楚了。编译前后产生新的文件对理解插件结构比较不友好

volatile-static commented 5 months ago

但现在是一部分在manifest.json中定义，另一部分有脚手架覆盖（version、ID等），迷惑性更强，还不如全整合 @windingwind

windingwind commented 5 months ago

替换字符串我觉得没啥问题呀，现在所有的html，css，pref.js等等都是替换字符串，本身这就是脚手架的功能。完全运行时生成的话，新手在编译之前完全不知道插件结构长啥样，也很难理解编译的过程发生了什么

volatile-static commented 5 months ago

这里我有两个问题 @northword

  1. 为啥要合并两次： https://github.com/northword/zotero-plugin-scaffold/blob/e776abc3742de281df6fcd7825a055417c278a35/src/lib/build.ts#L101-L102 https://github.com/northword/zotero-plugin-scaffold/blob/e776abc3742de281df6fcd7825a055417c278a35/src/lib/build.ts#L119

  2. 现在脚手架也没有处理manifest.json的i18n吧？写在config和写在manifest.json有什么区别呢？

volatile-static commented 5 months ago

我刚刚没表达清楚

  1. 我没有说移除对manifest.json文件的支持，只是想增加对config的支持
  2. 现在脚手架的替换方法是允许省略manifest.json文件中相应字段的，省略后会增加迷惑性
northword commented 5 months ago

确实多合并了一次；

现在没有处理 manifest.json 的 i18n ，用户需要自己准备 manifest.json 里可以本地化的内容。

version、id、update_url 直接覆盖掉是为了防止用户没有设置文本替换的占位符，name 可以略掉不做处理。

只是想增加对config的支持：是增加一个 config.build.makeManifest.template 类似的配置？

windingwind commented 5 months ago

个人意见，我是支持奥卡姆剃刀的，主要考虑是会增加后续维护成本，比起功能炫酷丰富我更关注长期维护的可行性。当然加config的支持增加的复杂性和提供的额外能力（在我看来应该没有额外的能力，只是另一个方法做到相同的事情）的性价比我不太了解，但总的来说每一处都增加，最终都会累积到后面的维护成本上去

关于省略字段，我觉得当作一个配置错漏的保险能力即可，确实不推荐省略，尤其是在插件模板中不会省略。既然已经有了，也不必要特意去掉这个功能

volatile-static commented 5 months ago

@northword

最开始的几版是整合了的，但是 i18n 不太好处理，所以就移除了。

@windingwind 我理解在你看来确实是没必要的，但有没有可能这属于个人习惯的范畴

volatile-static commented 5 months ago

我的习惯是尽量减少重复出现的代码，如果可以省略，那就一定要省略

windingwind commented 5 months ago

你省略的代码不都最后跑到这个包里来了嘛😂最终维护成本转嫁到其他人头上 总的来说我对具体功能不持意见，主要不管是谁（即使是你自用）要长期维护总归消耗精力的，整个包功能越多复杂性越高，精力不够停止维护的风险就越大。这个不像toolkit那种，各个模块分开，哪个接口用不了了，直接删了不维护也没事。构建流程耦合还比较高的，前期加的越多可能后续回转的余地就越小了

northword commented 5 months ago

既然也没有处理，那一开始说这个的原因是什么呢？我感觉跟这个没有关系呀

有关呀，如果要支持 i18n，就需要在 config 分 locale 填 manifest template，

如果是不支持 i18n ，在 config 里填一堆 __MSG_xxx__ 的话，我觉得和单独填区别也不是很大。

一开始就是用 config.build.makeManifest.template 直接生成 manifest.json，因为我没有做 分 locale 填 manifest template，所以改成了 只替换 name、version、id、update_url，因为这几个字段是强制性的（目的是为了兜底）。（name 是个意外，不应该覆盖，当用户填了 name 的时候，应该保持用户的）。

当然，加一个不支持 i18n 的 config.build.makeManifest.template 的话，我个人没什么意见，这个的维护成本并不高，也就加个判断和合并一次的区别。

volatile-static commented 5 months ago

有人用就会有人维护。

如果一个库不满足我的需求，我很可能会选择自己实现

windingwind commented 5 months ago

咱理性讨论别涂大字报，有人用=有人维护其实倒不一定的，像toolkit的prompt，升到115以后中间也断档了有一段时间。即使poly提的pr，poly自己插件也在用，当时也部分地失修了好几周。当然没有责怪谁的意思，大家都很忙，没有谁有责任即时维护

对于这个功能，如果是很方便的顺手修改，我觉得都ok；在我看来公用的库核心是取最大公约数，大家都用得到的部分共同维护，或者前人栽树后人乘凉。其中必然有关于接受或不接受哪些改动，我不是库拥有者，只是作为提出社区成员的一点想法

northword commented 5 months ago

emmm，看了下代码和 chartero 的构建代码，似乎实现起来并不复杂，也许可以做成 template 合并覆盖 已有的？

（现在代码里的template是完全没用的，属于漏删了

如果做支持多语言的话，可以不需要在template里填id version那些，只允许填name、desc那些？也许需要有个方案看看是否可行。

（这个应该还是挺稳定的，除非火狐大概插件）