I tried to perform all possible tests (however some translator seem broken). Please have a look at the suggested changes. I can update the lastUpdated afterwards in a bulk edit.

let won't work in any browser but Firefox, and there's not really a good way to make it work on other browsers, since one can't simply replace let with var. for ... of will work in newer Chrome and Safari versions, but it would break IE. I could add a regexp to convert for ... of to ordinary for loops in IE as we do right now for for each, but I'd rather just use ordinary for loops to begin with. We could run that regexp on all the translators now and get rid of all the for each uses, but the code it generates is pretty ugly.

Also, note that there are some intricate differences between for each and for of zotero/zotero#224 (comment)

Actually, I began with normal for before trying the "smarter" for ... of. I still have that work in my stash, e.g.

diff --git "a/C:\\Users\\zumstein\\AppData\\Local\\Temp\\TortoiseGit\\Age1E26.tmp\\Agencia del ISBN-cf42ad8-left.js" "b/C:\\Users\\zumstein\\AppData\\Roaming\\Mozilla\\Firefox\\Profiles\\766q0b3g.zotero-developing\\zotero\\translators\\Agencia del ISBN.js"
index 170b43a..06df1cb 100644
--- "a/C:\\Users\\zumstein\\AppData\\Local\\Temp\\TortoiseGit\\Age1E26.tmp\\Agencia del ISBN-cf42ad8-left.js"   
+++ "b/C:\\Users\\zumstein\\AppData\\Roaming\\Mozilla\\Firefox\\Profiles\\766q0b3g.zotero-developing\\zotero\\translators\\Agencia del ISBN.js" 
@@ -63,8 +63,9 @@ function doWeb(doc, url) {
        author = data['Autores'];
        if (author) {
            var authors = author.match(/\b.*,\s+\w+[^([]/g);
-           for each (aut in authors) {
-               item.creators.push(Zotero.Utilities.cleanAuthor(Zotero.Utilities.trimInternal(aut), "author", true));
+           for (var i=0; i<authors.lenght; i++) {
+               var aut = Zotero.Utilities.trimInternal(authors[i]);
+               item.creators.push(Zotero.Utilities.cleanAuthor(aut, "author", true));
            }
        }
        if (data['Publicacin']) item.publisher = Zotero.Utilities.trimInternal(data['Publicacin']);

Would that be nicer than the automatic replacement and compatible along all browsers?

Yes, that would be better (except for the typo in length).

Okay, I can work on that further manually.

I just updated the first ones, i.e.

• using a simple for (for all cases where the variable is an array and indexed by numbers).
• use var instead of let
• check that there is no other variable within the function scope wit the same name
• run test cases whenever possible
• fix some spacing
  I would like to continue in this way for these cases.

Okay, here are the other cases, except the three RDF translators (and IEEE Xplore translator). Is this okay for you to review or do you wish some other action before?

Some more for eachs are in the RDF and TEI translators.

I would suggest to sqash now to have one clean commit for you to review?

@simonster @aurimasv Any opinion here? How to proceed?

just dropping in quickly to point out that some of these translators are entirely defunct (e.g. Ancestry afaik) so it doesn't make any sense to fix details of them.

@adam3smith When do you delete a translator completely from the repo? An "entirely defunct translator" sounds for me like we should delete that. When there is some interest in the future, it might be possible to write a new one.

we delete translators when sites don't exist anymore or translators are merged. We don't delete translators that are completely broken if there's any chance they'll be replaced by functioning versions in the future.
That's at least what we've been doing so far--I don't have a strong opinion on whether that's a good policy or whether it'd makes sense to delete more. Once consideration might be that even where sites change dramatically, mappings and clean-up code may still be useful.

Hm... it would at least be good to "mark" such completely broken translator. If "code may still be useful" then I suggest also to do these refactoring on this code as well.

@aurimasv I updated the PR as you requested. Please have a look at the new version.

Thanks!! (though I probably should have squashed those commits on merge. Ooops)

Okay. Cool. Merging would have been done a cleaner git history, but in this way my stats (+9 commits) are evolving faster ;-)

Now, we only should have for each loops in the three RDF translators and in the TEI translator. They seem to be more tricky, because they involve cases with objects which are not arrays.

Should be able to do a straightforward replacement from for each(var i in obj) { to

for (var j in obj) {
  var i = obj[j];

Though I haven't looked at those cases in particular