PicGo-Core icon indicating copy to clipboard operation
PicGo-Core copied to clipboard

PICGO_ENV should not be coupled in picgo-core

Open upupming opened this issue 2 years ago • 6 comments

Currently in picgo-core it will see if it is 'GUI' mode (in PicGo Electron) and look at different path for the clipboard scripts.

https://github.com/PicGo/PicGo-Core/blob/f133d57562c413b0b6f9a9ca9a93bf19c1768f1f/src/utils/getClipboardImage.ts#L33-L43

In my opinion, picgo core is just a library, it should not handle this kind of "hack" by itself. In the past, this special hack also affects vs-picgo: https://github.com/PicGo/vs-picgo/issues/75

Also, vs-picgo has to do something like "copy the clipboard" files when bundling itself. See:

https://github.com/PicGo/vs-picgo/blob/f81a1dd7eaa5daf4be2b7329d92e9abf829892b7/esbuild.mjs#L96-L99


I think all these problems can be solved by just read these scripts into bundle by something like raw-loader. And write to a script when needed before running the script. In this way, all the scripts can be "bundled" into picgo-core with no trouble. And no one needs to handle the scripts files any more.

I have solved this in #102

upupming avatar Oct 05 '21 08:10 upupming

PICGO_ENV will be used in many ways, such as GUI\CLI\WEB. But now it comes from ctx.getConfig which is a bad option. It will be used in plugins.

inline-loader is a good solution for these scripts, I think you should just open a PR for inline-loader, I will take care of PICGO_ENV

Molunerfinn avatar Oct 09 '21 03:10 Molunerfinn

@Molunerfinn It seems the only usage of PICGO_ENV in picgo-core now is just to handle the clipboard script files, as I have used inline loader to load this scripts in PR #102, this PICGO_ENV is no longer used in picgo-core. It seems the PICGO_ENV is written to config file by PicGo Electron but not PicGo-Core.

upupming avatar Oct 09 '21 06:10 upupming

If we want to support picgo to different platforms, the PICGO_ENV will be a key for plugins to get

Molunerfinn avatar Oct 09 '21 06:10 Molunerfinn

For now, it may not be used in code, but would be used in the future

Molunerfinn avatar Oct 09 '21 06:10 Molunerfinn

@Molunerfinn

If we want to support picgo to different platforms, the PICGO_ENV will be a key for plugins to get

Like your idea, this env will make plugins be aware of current picgo environment, it is super cool! But I think we should not save this PICGO_ENV in the configuration file but determine it at runtime. This is because:

  1. all environment web/cli/gui should be able to read an existed configuration file
  2. picgo electron depends on picgo-core, and picgo-core has cli integration, too. Actually picgo electron can still run cli mode if it wants.

upupming avatar Oct 09 '21 06:10 upupming

Yes, as I said, it comes from ctx.getConfig which is a bad option, it will be a param of ctx, that would be better.

Molunerfinn avatar Oct 09 '21 06:10 Molunerfinn

done

Molunerfinn avatar Nov 15 '22 14:11 Molunerfinn