Closed jjspace closed 1 year ago
Great work! Looks like a very nice addition. Will test a little more and then publish shortly if everything looks good
@mjbvz I'm sorry for accidentally creating some extra issues with my changes and making you do extra cleanup (https://github.com/mjbvz/vscode-github-markdown-preview-style/issues/104, https://github.com/mjbvz/vscode-github-markdown-preview-style/issues/103). Can you elaborate a little on why your fixes were needed/the better changes in these cases? (e0286fd5580cf39704a3d3a6666919573c1b04de b2adb1fbadda6957888345b32a55594f218746a5, d01f1e7a1d39f4210572086132ea53c9e255f5e0) Just so I can learn and don't repeat mistakes in the future as I've never worked on vscode extensions before (and "it works fine on my machine" is not a great excuse :laughing:)
ps. using this pr just to avoid excess noise for other users in the issues, wasn't sure the best place for a discussion
No worries, the fixes were pretty quick and easy (and the more important break was my fault)
To recap. The first issue was that .cjs
doesn't seem to work on web. I fixed that but in the process broke this extension on desktop because I forgot to commit the change that also removed type: "module"
in the package.json
. type: "module"
caused the extension.js
script to be loaded as a module, which was now failing because it's commonjs and no longer had a explicit .cjs
extension to tell node otherwise. Then finally I had to make sure the build script was treated as a module again by converting to an .mjs
file
Probably not a common issue. Will see if I can add a browser debug config to the extension so you can more easily test the extension in browsers
Summary
generate-github-css
to include options for all theme types in a new generation scriptdata
attributes instead of a single class name to allow for easier mixing and matching. (This matches up closely with how Github themselves does it)type: module
to allow working withgenerate-github-markdown
which is an ESM module not a CommonJS module. This also required changing theextension.js
toextension.cjs
to continue working as it used toI believe the way the new settings are set up will preserve the existing behavior for anyone who had modified them before.
Resolves #89 Resolves #75
Please let me know if you have any questions!
I don't know much about actually publishing extensions so I believe all my changes should work for those who install them but please verify for me so I'm not accidentally breaking this for a bunch of users (myself included)