Ceres Posted September 6, 2021 Report Share Posted September 6, 2021 Do you use e.g. ESLint, when you write new or edit JS code? Thanks for sharing your experience! Quote Link to comment Share on other sites More sharing options...
Stan` Posted September 6, 2021 Report Share Posted September 6, 2021 We use ESLint the config file is there https://trac.wildfiregames.com/browser/ps/trunk/build/arclint/configs/eslintrc.json If you use arcanist it will try to run ESlint. On windows it's a bit of a mess, since you have to npm install eslint in the root folder of 0ad. 1 Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) @Stan`Thank you so much for your lightspeed and helpful reply. You must have mind-reading abilities, too, as I just wanted to ask how to answer 'npx eslint --init'. But now I can just use the already available config file you pointed me at. 2 furthers questions: 'npx eslint --init' creates .eslintrc.js, instead of eslintrc.json. Shall I rename it? To which? This remains unclear to me after reading the README.md, but I can just try which works. I do not use arcanist (yet). Is it recommended for me when I (currently) just want to contribute to a few simple patches? Can I use the external ESlint without arcanist? (this was my understanding) PS: I stopped trying to build/help with patches under Windows 10, as it's indeed overall a pain in my back. Edited September 6, 2021 by Ceres Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) Oh, well, the GUI editors are annoying, at least when it comes to lint implementation, so I'm going to get arcanist - phew... @Stan` Understanding that I can install arc anywhere (as long as I add it to my PATH environment variable), do you have some general recommendation where to put it? Right under e.g. ~/arc/ (/home/user/arc/in my case)? Edited September 6, 2021 by Ceres Quote Link to comment Share on other sites More sharing options...
Stan` Posted September 6, 2021 Report Share Posted September 6, 2021 Why not install it through the debian repos ? Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) Erm, what to install through the debian repos? EDIT: Do you mean arc itself? I read in the linked instructions to install arc via git. Should I remove it and re-install from the repo instead? Would be "cleaner", I guess. /EDIT Anyway, I now got arc running (installed via git), and the API token is set. Now I read: Quote Download a patch To download a patch from Phabricator using Arcanist, run the following command from the root of a local SVN clone (where .arcconfig is): arc patch Dn This will apply the latest diff from a revision Dn (n being an integer) to your working copy. Since I'm now interested to amend my latest patch wrt missing spaces (and thought I could do this through arc+lint, lint just to check), I'm currently unsure if I'm still on the right track. Edited September 6, 2021 by Ceres Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) Ok, installed arcanist via its Debian repo - much better. I edited the Wiki on Phabricator by adding this: Quote Note: You can install arcanist also via e.g. the Debian repo: sudo apt install arcanist Edited September 6, 2021 by Ceres Quote Link to comment Share on other sites More sharing options...
nwtour Posted September 6, 2021 Report Share Posted September 6, 2021 (edited) 2 hours ago, Ceres said: when you write new or edit JS code I may be mistaken, you are confusing the editor and the linter. Linter is just a checker, he writes comments on the screen and that's it. JavaScript is edited in any simple text editor - like a notepad Edited September 6, 2021 by nwtour Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) No, no, I see the difference. But when I edit some JS file in the editor, I would like to avoid later seeing complaints from the linter about my patch. I saw that there are linters that can show problems on-the-fly, i.e. while typing, whereas others need to be fired after the file has been saved. Therefore, I asked all these questions. Does that make sense to you? BTW, I already have syntax highlighting for JS enabled, which helps reading it. Edited September 6, 2021 by Ceres Quote Link to comment Share on other sites More sharing options...
nwtour Posted September 6, 2021 Report Share Posted September 6, 2021 16 minutes ago, Ceres said: I see the difference Ok Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) Now I wonder where the eslintrc.json config should go to - user's home or project folder? Besides, arc seems to work: user@Debian:~/0ad$ arc list * Needs Review D4252: GUI Enabled Mods - short explanatory note added about later mods overwriting earlier installed mods * Accepted D4254: Added translatability of message "%(unit)s can't be controlled." Cool, eslint in arc works, too: user@Debian:~/0ad$ arc lint >>> Lint for binaries/data/mods/public/simulation/helpers/Commands.js: Error (ESLINT) no-use-before-define 'g_Commands' was used before it was defined. 46 // moves the entities closer to the target before giving up.) 47 48 // Now handle various commands >>> 49 if (g_Commands[cmd.type]) ^ 50 { 51 var cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger); 52 cmpTrigger.CallEvent("OnPlayerCommand", { "player": player, "cmd": cmd }); Error (ESLINT) no-use-before-define 'g_Commands' was used before it was defined. 50 { 51 var cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger); 52 cmpTrigger.CallEvent("OnPlayerCommand", { "player": player, "cmd": cmd }); >>> 53 g_Commands[cmd.type](player, cmd, data); ^ 54 } 55 else 56 error("Invalid command: unknown command type: "+uneval(cmd)); Warning (ESLINT) object-curly-spacing A space is required after '{'. 974 "type": "text", 975 "players": [player], 976 "message": markForTranslation("%(unit)s can't be controlled."), >>> 977 "parameters": {"unit": cmpIdentity.GetGenericName()}, ^ 978 "translateParameters": ["unit"], 979 "translateMessage": true 980 }); Warning (ESLINT) object-curly-spacing A space is required before '}'. 974 "type": "text", 975 "players": [player], 976 "message": markForTranslation("%(unit)s can't be controlled."), >>> 977 "parameters": {"unit": cmpIdentity.GetGenericName()}, ^ 978 "translateParameters": ["unit"], 979 "translateMessage": true 980 }); The experts might yawn over this, but for me it's nice to see some progress. It's CLI and not a GUI editor, but that's ok. Now before I forget over all this, I need to go back and check my patch for the missing spaces in https://code.wildfiregames.com/D4254. BTW, can we ignore the error messages? (I have no clue, sorry) Edited September 6, 2021 by Ceres Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 Hmm, the new revision of patch D4254 is 18576, arc tells me, but I cannot see 18576. What have I done? https://code.wildfiregames.com/D4254?id=18576 user@Debian:~/0ad$ arc diff --only You have untracked files in this working copy. Working copy: /home/user/0ad/ Untracked changes in working copy: (To ignore these 15 change(s), add them to "svn:ignore".) Commands.js.patch binaries/system/ActorEditor binaries/system/pyrogenesis binaries/system/test build/premake/.gccmachine.tmp build/workspaces/gcc libraries/source/fcollada/.already-built libraries/source/fcollada/src/output libraries/source/nvtt/.already-built libraries/source/nvtt/src/build libraries/source/spidermonkey/.already-built libraries/source/spidermonkey/include-unix-debug libraries/source/spidermonkey/include-unix-release libraries/source/spidermonkey/mozjs-78.6.0 source/lib/res/graphics/tests Ignore these 15 untracked file(s) and continue? [y/N] y Linting... >>> Lint for binaries/data/mods/public/simulation/helpers/Commands.js: Error (ESLINT) no-use-before-define 'g_Commands' was used before it was defined. 46 // moves the entities closer to the target before giving up.) 47 48 // Now handle various commands >>> 49 if (g_Commands[cmd.type]) ^ 50 { 51 var cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger); 52 cmpTrigger.CallEvent("OnPlayerCommand", { "player": player, "cmd": cmd }); Error (ESLINT) no-use-before-define 'g_Commands' was used before it was defined. 50 { 51 var cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger); 52 cmpTrigger.CallEvent("OnPlayerCommand", { "player": player, "cmd": cmd }); >>> 53 g_Commands[cmd.type](player, cmd, data); ^ 54 } 55 else 56 error("Invalid command: unknown command type: "+uneval(cmd)); LINT ERRORS Lint raised errors! Running unit tests... No unit test engine is configured for this project. SKIP STAGING Phabricator does not support staging areas for this repository. Created a new Differential diff: Diff URI: https://code.wildfiregames.com/differential/diff/18576/ Included changes: M binaries/data/mods/public/simulation/helpers/Commands.js I'm in the wrong thread but will move this later. Quote Link to comment Share on other sites More sharing options...
Stan` Posted September 6, 2021 Report Share Posted September 6, 2021 Well it's here https://code.wildfiregames.com/differential/diff/18576/ To update an existing diff you can do arc diff --update DXXXX https://secure.phabricator.com/book/phabricator/article/arcanist_diff/ 1 Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 I had (wrongly?) understood that 'arc diff --only' would ask me if to create a new one or update an existing one: Quote Upload a patch arc diff --only creates a diff from your working copy and uploads it to Phabricator. When following the given link, you will be able to create a new revision or update an existing one with the new code change. Therefore, I simply executed 'arc diff --only', i.e.without specifically providing the D No. for the patch. As it seems, I have not messed everything up, though Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) Why does https://code.wildfiregames.com/D4254 not show 18576? (despite it's there, when you provide this link: https://code.wildfiregames.com/differential/diff/18576/) Edited September 6, 2021 by Ceres Quote Link to comment Share on other sites More sharing options...
Stan` Posted September 6, 2021 Report Share Posted September 6, 2021 Because when you do diff --only it does not attach the diff automatically to the revision You need to do it through that interface. 1 Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 6, 2021 Author Report Share Posted September 6, 2021 (edited) Ok, I got it! And I have learnt again something new. Thanks @Stan` and others who are so patient with me. Sorry that I got/get OT here, but do we have to define g_Commands first before it's used in that code part (Commands.js)? Edited September 6, 2021 by Ceres 1 Quote Link to comment Share on other sites More sharing options...
Stan` Posted September 6, 2021 Report Share Posted September 6, 2021 Don't worry about that one. It requires a bigger refactoring. 1 Quote Link to comment Share on other sites More sharing options...
Feldfeld Posted September 6, 2021 Report Share Posted September 6, 2021 (edited) I know emacs + flycheck (maybe also flymake) does it on the fly, idk about others. Edited September 6, 2021 by Feldfeld 1 Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 7, 2021 Author Report Share Posted September 7, 2021 Thank you both. May solution now is to edit a JS file, let arcanist check it with its integrated linter, correct what needs correction, and upload it to phabricator. The arc linter messages in the CLI are sufficient for my humble needs at the moment. Quote Link to comment Share on other sites More sharing options...
Loki1950 Posted September 7, 2021 Report Share Posted September 7, 2021 Almost all of the text editors available on Linux have syntax highlighting which can be set for a very long list of programming languages with both C++ and JavaScript just being two. Enjoy the Choice Quote Link to comment Share on other sites More sharing options...
Ceres Posted September 9, 2021 Author Report Share Posted September 9, 2021 On 06/09/2021 at 2:52 PM, Ceres said: ... BTW, I already have syntax highlighting for JS enabled, which helps reading it. My question was about linting (not highlighting), but I am set now. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.