Just a side-note -- --fix doesn't work for the translators because of eslint/eslint#5121.

(of course a gnarly workaround can be made by physically splitting the translators into header and code files, linting them separately and recombining them if --fix is ran, but that's a little uglier than I'm comfortable with. So far, with the use of vim, bringing code in line was a few minutes per translator (hence the barrage of PRs))

Oh and personally I'm all for a high level of encouraged consistency in the translators. Given that most new translators seem to be copy-and-change jobs from existing translators, enforcing rules would be low-impact (since they'd be copying fairly clean code after the first major sweep) and it'd not further proliferate bad code styles.

Just a side-note -- --fix doesn't work for the translators because of eslint/eslint#5121.

Oh, that's annoying. I guess not making these proper JS files was shortsighted, but we're at least a decade out from that decision, so we're probably stuck with it.

of course a gnarly workaround can be made by physically splitting the translators into header and code files, linting them separately and recombining them if --fix is ran, but that's a little uglier than I'm comfortable with.

I don't know — it seems fairly trivial to just have a script that splits a translator, runs --fix on a temp file with the code part, and merges back to the original file.

It is annoying, and we're also stuck with it. I dunno -- it wouldn't be hard to make a script that just does what the preprocessor does and undoes it after. But it'd be changing files on disk, inside the repo. If it bombs out, it would leave changed files which could accidentally be checked in.

I'm not really concerned with accidental commits — anyone committing should be reviewing the diff first, and --fix itself has to be reviewed anyway — but it should be easy to make the script fairly safe.

I can have a stab at that.

I'll just do the split to a separate directory and stick that in .gitignore, that should minimize the risk.

I'll wait for #1895 to clear though, it would build on some of those changes.

I'll just do the split to a separate directory and stick that in .gitignore, that should minimize the risk.

That's not necessary. The header can just stay in memory, and the code can go in a temp file, and then they can be recombined back into the original file. The temp file can be cleaned up in an exit trap in case something goes wrong.

You mean by running eslint from the script itself? Wouldn't this split off the messages about the header from the rest of the eslint output? Just adding the temp dir to .gitignore would achieve much the same and we could just run eslint as usual.

Yes, we'd just have an eslint-fix script in .ci that took a filename. This is just for running --fix, so the header checks wouldn't be relevant.

I don't completely follow, but I think we agree on what we want to achieve and roughly how to do it.

Oh, that's annoying. I guess not making these proper JS files was shortsighted, but we're at least a decade out from that decision, so we're probably stuck with it.

Are we though? What would be the harm from optionally allowing a translator to start with const ZOTERO_TRANSLATOR_INFO = and stripping this exact string from the start of translators when they are loaded into Zotero? Old translators would continue to work, and an ESLint rule to detect that it's missing would be easy enough to do.

I don't think it's worth it. We'd have to make changes in many places in many repos, we'd have to either cut off translator updates or send backported translators to older versions, and we'd risk bugs in critical infrastructure, all with very little to be gained. We already have your postprocessor to get ESLint checks working, and making a --fix wrapper would take about 10 minutes.

I have a wrapperless solution that I'm testing now.

Alright, the results are at https://github.com/retorquere/translators/tree/eslint-split-merge

It looks a little baroque, I know, but the current implementation achieves a few goals I think are interesting:

• First of all, it allows --fix
• It does this while retaining the processor. Retaining the processor means that vscode et al can load the eslint config -- otherwise they would error out on the header.
• No temporary files
• Monkey-patches notice/notice so it can fix a missing license comment

Fix inside editors is not (currently) possible because they must load the plugin processor.

Are we though?

I had forgotten that there are more environments than just the Zotero client that load the translators.

Are the test cases at the end required to be valid JSON or just valid JS?

Are the test cases at the end required to be valid JSON or just valid JS?

JSON, from the looks of it, which doesn't really make sense, but they're only ever generated by Scaffold anyway.

That's ok but then the fixer workaround needs to protect them. I'll add that.

It's not that bad an idea -- JSON can be parsed safely using JSON.parse. Otherwise you'd have to get fancy with either sandboxes or esprima or whatnot.

I've merged the changes into #1895, and added two test for testCases. Fix now works without touching the header or the testCases.

I have some time available this week and I'd love to plow through the translators based on this. I've done some test and after fix (which I did mostly manually so far), the linter reports quite a few genuine errors.

My weekend looks (reasonably) good, so I can help speed-review & merge.

I take it it is still best if I do one pr per translator? Or do you want them batched at any size?

Unless it's much more work, one per translator is easiest in case there are blockers/questions, so if it doesn't make a difference to you, do that. If you find it takes extra time, batches of 5 are fine, too. And thanks!

Nope, I have a few scripts to manage this process, one per is no problem whatsoever.

What about the existing PRs? Squash or submit new ones?

squash if you don't mind. (you don't even need to squash now that github allows me to do that on merge)

No problem. I know what to do now.

Oh if I don't have to... that saves me updating my scripts.