search

Benjol / SE-AutoReviewComments

AutoReviewComments - Pro-forma comments for SE
http://stackapps.com/q/2116
98 stars 39 forks source link

Can the userscript's Local Storage data be put into a single property, rather than 30 or more separate ones? #175

Open CertainPerformance opened 4 years ago

CertainPerformance commented 4 years ago

I pretty frequently look through SE sites' Local Storage when debugging my own scripts. Every time I do, I'm faced with screens like this:

localStorage pollution

AutoReviewComments' keys are everywhere - there are about 60 of them for me (depends on how many auto-comments one has saved). It's not only annoying, it's also a pretty inelegant storage pattern. A better choice would be to use a single property instead. For example, rather than AutoReviewComments-commentcount, AutoReviewComments-WelcomeMessage, AutoReviewComments-desc-6, AutoReviewComments-name-6, AutoReviewComments-LastUpdateCheckDay, a single object could be used instead:

{
  welcomeMessage: 'some string',
  LastUpdateCheckDay: 12340000,
  templates: [
    {
      name: '[A] Answers just to say Thanks!',
      description: 'Please don\'t add "thanks" as answers. .....'
    },
    {
      name: '[Q] Some template name for question comment',
      description: 'Some description'
    },
    // ...
  ]
}

JSON.stringify can be used to save such an object in storage, and JSON.parse can turn the JSON string back into an object:

var db = JSON.parse(localStorage.AutoReviewCommentsSettings);

I would love if I could make a pull request to change AutoReviewComments' storage to the above structure. Would this be an acceptable change for me to work on?

This would still be completely backwards-compatible: the script can check to see if the individual AutoReviewComments keys exist, and if they do, they could be extracted to construct the new settings object. The modifications would still be able to read settings in either the individual format or consolidated format, but would only save settings to a single key.

oliversalzburg commented 4 years ago

I believe the code comes from a time when there were storage limits per key in the LocalStorage implementations, if there weren't even other restrictions or storage patterns at the time.

Either way, the change seems perfectly reasonable, but the code hasn't been maintained in years and if new releases will ever be built is questionable.

If you want to work on it, you're welcome to do so, but hopefully you won't rely on support from original maintainers, because we haven't worked on this in a long time and I personally refuse to work on anything related to that cancerous, hateful platform and company.

Benjol commented 4 years ago

If you do the work and send a pull request I don't mind having a look at it, but I'm not 100% sure I still have the access rights to update the chrome extension on the store. Nor that it still corresponds to their criteria.

On Wed, 29 Apr 2020 at 09:33, Oliver Salzburg notifications@github.com wrote:

I believe the code comes from a time when there were storage limits per key in the LocalStorage implementations, if there weren't even other restrictions or storage patterns at the time.

Either way, the change seems perfectly reasonable, but the code hasn't been maintained in years and if new releases will ever be built is questionable.

If you want to work on it, you're welcome to do so, but hopefully you won't rely on support from original maintainers, because we haven't worked on this in a long time and I personally refuse to work on anything related to that cancerous, hateful platform and company.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Benjol/SE-AutoReviewComments/issues/175#issuecomment-621037351, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACE6AWTCU7NGQWJUU6JEWLRO7J4HANCNFSM4MTN2QLQ .