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

chore: change build outfile to addonRef.js #66

Merged

Conversation

northword
Copy link
Collaborator

将 edbuild 的输出文件名从 index.js 更改为${config.addonRef}.js,这对维护多个插件的开发者应是有利的,例如在控制台,开发者看到的就不是 index.js 了,这样不同插件打印的日志就比较好区分。

(部分时候并没有用 ztoolkit.log而是直接用了console.log;此外有部分插件开发和发布过程并没有区分既有的 dev 和 prod,改的时候不容易区分)

Snipaste_2023-07-17_17-05-59

@windingwind
Copy link
Owner

我感觉这一个更改的必要性可能值得商榷,用文件名来区分不同插件可能不是太好的做法。

  1. 用文件名区分插件的合理性存疑。显示文件名只是console的副作用,在prod模式下,用户无法获取文件名,因此无助于调试排错。
  2. 用文件名区分插件的必要性存疑。一般情况下用ztoolkit.log替代console.log就可以达到区分输出来源的目的。实际上应该尽可能不要直接用console,而是将输出目的地根据插件构建类型(dev,prod)自动判断,因为用户完全无法获取console的输出,只会降低运行效率。
  3. 不同插件编译后产生不同的主入口容易造成新手困惑(为什么我的插件和模板编译的入口不一样?)

欢迎探讨

@northword
Copy link
Collaborator Author

以下是我的观点:

1与 2 可以合并考虑,

  • 都是仅便于开发时区分,因为 ztoolkit.log 并不是 Zotero 提供的,所以不是所有的插件都使用了 ztoolkit.log。
  • 不是所有的插件都区分 dev 和 prod,所以部分插件的发布版本仍有大量 console 输出
  • 即使是 ztoolkit.log,有时候也会输出没有插件前缀的日志(这个很奇怪,我还没有找到原因,似乎是 prompt 产生的,即便插件实际没有注册 prompt)

当然这个可以通过改这些插件,开发时仅启用一个插件(虽然这本就是推荐的做法)来解决,改文件名确实只是一个妥协做法

以下是一些例子:

Snipaste_2023-07-18_18-28-20

Snipaste_2023-07-18_18-23-28
Snipaste_2023-07-18_18-25-27
Snipaste_2023-07-18_18-26-35
Snipaste_2023-07-18_18-27-37

对于 3,个人觉得新手可能不会考虑到这个。而且插件的主入口应该可以算是 bootstrap.js ,这一文件 startup 中写了 load 哪一个脚本,新手开发者理解了插件加载流程后应不会对这个产生疑惑。(如果真要产生疑惑,那可能就多了,比如如果不清楚 rooturi,那就会疑惑大家插件都是 index.js,安装多个后不会被覆盖吗)(不过刚把模板改了一点就跑不起来了的话,确实很有可能会怀疑到这个上来)

此外,插件 better-bibtex 是一个使用插件名作为文件名的已有例子:https://github.com/retorquere/zotero-better-bibtex/blob/8d743a720c67a43911e003c34f251dc6c6becfe0/content/bootstrap.ts#L69

@volatile-static
Copy link
Contributor

我支持windingwind的第二点,其他2点支持northword。其实我从一开始就使用chartero.js而不是index.js

@windingwind windingwind merged commit efd56c9 into windingwind:main Jul 19, 2023
@northword northword deleted the change-build-outfile-to-addonRef branch December 10, 2023 14:14
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.

3 participants