-
Notifications
You must be signed in to change notification settings - Fork 40
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
Modernize extension #38
Conversation
And use arrow functions to adjust it for safeUpdateDOM
w00t! This is a large rewrite! Thanks for bringing the extension to the 21st century! :) I'll test it out with Firefox later today. I'm assuming that you've done Chrome already. I'll leave some comments on the PR as well. |
Adding a note for later review: Userscript version. |
Yeah, about the userscript version. Are you sure you want to maintain that? Especially manually? I've been asked to port my extensions to userscripts before but userscripts aren't as capable and I don't really care of userscripts (which need a base extension to be executed in the first place, like tampermonkey and greasemonkey) |
There is an argument to be made about ubiquity of userscripts: they can run on IceMonkey/Opera/Safari, etc almost out of the box. However, I also see that the Userscript may restrict one to an older version of JavaScript. Perhaps @yfdyh000 could also share some of his motivations for making a Userscript version in the first place? |
Sure, if you want to support the dozen of users on those three browsers combined no one will stop you, I just don't think it's worth the effort. Opera supports Chrome extensions anyway. I'll add: it's not about javascript versions but about the web-ext API, like the more flexible Storage API |
I don't think the userscript block this review, you can keep it as a legacy version. Create userscript because the original extension is almost just a userscript, missing meta only, and I hope it can be more easily customized and improved. |
Also, I think this updates can be modified to userscript, but there is no significant power to do so for now. |
I've merged this in and have left the userscript as the legacy version. However, I'll release the next version after I've added the preference for the indentation. Thanks @bfred-it! |
No description provided.