Bug 456493

If someone is up for it, this should be easy to implement.

(In reply to Waqar Ahmed from comment #1)
> If someone is up for it, this should be easy to implement.

I wish I even knew where to begin; I think this would be a very useful feature to a lot of people.

An MR is already in review which includes basic ESLint integeration. It would be great if you could test :)

https://invent.kde.org/utilities/kate/-/merge_requests/1051

(In reply to Waqar Ahmed from comment #3)
> An MR is already in review which includes basic ESLint integeration. It
> would be great if you could test :)
> 
> https://invent.kde.org/utilities/kate/-/merge_requests/1051

That's great! I have been attempting to figure out how to test - I got as far as `kdesrc-build --include-dependencies kate` on https://kate-editor.org/build-it/, but I don't know how to switch to and compile the branch with this feature. Is there some documentation on this I can refer to?

Switch to the folder which contains Kate source code. Then run

git fetch

It will fetch all branches. Then switch to the branch in the merge request you want to test

git checkout work/diagnostics

Then run kdesrc-build again like

kdesrc-build --no-src kate

--no-src means to not update to the latest source code because you are testing something locally.

https://docs.kde.org/trunk5/en/kdesrc-build/kdesrc-build/index.html

https://community.kde.org/Get_Involved/development#Building_software_with_kdesrc-build

(In reply to Waqar Ahmed from comment #5)
Thanks for your help; I was able to compile and run the branch. 

It took me a little time and reading through your commits to understand that I needed to go to Project > Code Analysis > Start Analysis to have ESLint run on my file*. Once I hit Start Analysis, everything seemed to work pretty smoothly, except that error messages appeared to be duplicated in the Diagnostics output (see attachment). I noticed live output in the panel from the LSP server, and ESLint output upon save; line highlighting worked as expected. 

I very much appreciate your work on this; it will really make life easier!


* You don't really benefit much from a full-on IDE for webdev, so some of the workflows in Kate that (presumably) come from KDevelop have taken some getting used to for me. It took me ages, for instance, to learn that I could use the "Build" tool to run an Eleventy / SSG server, but given the way RAM gets eaten when I do this, I'm not sure whether I ought to be using it this way.

Created attachment 155100 [details]
ESLint Error Duplication in Diagnostics Output

Thanks for the feedback, it helped find a critical issue.

I have removed "Eslint current file" from the "Code Analysis" tab and replaced it with "ESLint" which will run on the whole project instead of just the current file. Having the single file in projects + executing it on save complicates that plugin too much.

Additionally, I have added a new plugin named "ESLint" which will run only on the current file on saving. It doesn't support auto-fix yet though. So, you need to enable this plugin.

(In reply to Waqar Ahmed from comment #8)
> I have removed "Eslint current file" from the "Code Analysis" tab and
> replaced it with "ESLint" which will run on the whole project instead of
> just the current file. 
This makes sense. I imagine many users wouldn't think to enable the projects plugin, especially for simple web projects.

For testing purposes, I created a directory containing two JS files: 

Kate ESLint Test/
├─ test.js
├─ test2.js

File content:
-------------------------------------------
const count = (n) => {
	for (let i=1; i<=n; i++) {
		console.log(i);
	}
};

count(10);
-------------------------------------------

When I used File > Open Folder and clicked Project > ESLint > Start Analysis with no errors in either .js file, ESLint failed to run with the following error in the Output panel: 

[16:40:36  CodeAnalsis Error] Analysis failed with exit code 0, Error: [{"filePath":"/home/user/Kate ESLint Test/test.js","messages":[],"suppressedMessages":[],"errorCount":0,"fatalErrorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0,"usedDeprecatedRules":[]},{"filePath":"/home/user/Kate ESLint Test/test2.js","messages":[],"suppressedMessages":[],"errorCount":0,"fatalErrorCount":0,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0,"usedDeprecatedRules":[]}]

When I introduce an error into each file (an extra semicolon on line 4) and run Project > ESLint > Start Analysis, the error is output into Diagnostics output for test.js, but not test2.js. 

When I introduce the same error only to test1.js and run Project > ESLint > Start Analysis, everything behaves as expected.

When I introduce the same error only to test2.js and run Project > ESLint > Start Analysis, no error is sent to Diagnostics, but ESLint does not fail to run per the below output from the Output panel:

[16:56:57  CodeAnalsis Log] Analysis on ESLint files finished.

> Additionally, I have added a new plugin named "ESLint" which will run only
> on the current file on saving. 
The duplicated error issue looks resolved in the ESLint plugin. I do find that errors aren't removed from Diagnostics output when correcting the error and saving (is this what you meant by auto-fix?). The error is removed from Diagnostics output when a new error (from the same file) is sent to Diagnostics. This also applies to line highlighting.

I also notice that the log messages in the Output panel contain a typo: CodeAnalsis should be CodeAnalysis.

I think all these problems should be fixed now.

> many users wouldn't think to enable the projects plugin, especially for simple web projects

That is okay, but I would still recommend enabling it always as a lot of kate stuff works better if there is a project around e.g., search & replace inside a project

By auto-fix I mean for some diagnostics a source can provide a "fix". From the cmd line you would do eslint --fix, and it would fix stuff for you. This is also supported now. For diagnostics that are fixable there will be a "(fix available)" suffix at the end of the line. Once you double click the line or invoke the QuickFix shortcut(Ctrl + Period), it will fix that diagnostic. However, it might not work very well if there are multiple fixes available for multiple diagnostics.

(In reply to Waqar Ahmed from comment #10)
> I would still recommend enabling it always as a lot of
> kate stuff works better if there is a project around
Yea, I do find myself making use of it lately. I like the notes. 

> By auto-fix I mean for some diagnostics a source can provide a "fix". From
> the cmd line you would do eslint --fix, and it would fix stuff for you. 
Tested this - love it. I had no idea such a thing existed. 

All issues with the ESLint plugin look resolved for me, but I still have the same issues when running from Code Analysis. Maybe It's the way I set up the project? I just used "Open Folder" to open a directory containing two .js files; there's no git repo or project file like there would be in KDevelop. Do I need one of these things for it to function correctly?

> I still have the same issues when running from Code Analysis

Right. I forgot to push the change for that. Exit Code 0 wasn't being considered as a success code so it was erroring out. Should work now I think. Besides that, you don't need/have to setup a project to make this work. We do have a .kateproject file, but it is optional to use.

There is still the issue that if there is no eslintrc file or eslint is not initalized for a project, it will error out. ESLint doesn't seem to provide a way to add a fallback configuration that it could fallback to in case nothing was found locally. It is possible to add it explicitly, so perhaps we can support this in future in the ESLint plugin.

(In reply to Waqar Ahmed from comment #12)
> Exit Code 0 wasn't being considered as a success code so it 
> was erroring out. Should work now I think. 
Yep! This is no longer occurring. Running from the Project panel still doesn't output ESLint errors for test2.js though, only test.js. 

> There is still the issue that if there is no eslintrc file or eslint is not
> initalized for a project, it will error out.
I really like the idea of a fallback config in future (and maybe even an interface to customize it in the plugin settings); maybe a simple error message would be helpful for now? Something like "No ESLint configuration found. Ensure that ESLint is installed and initialized. See (URL) for more information.

Speaking of ideas for the future: it would be amazing to be able to have it work live; generating error messages without needing to save.

If you want, and the MR is accepted, I could help write user documentation on the plugin for The Kate Handbook?

Git commit 7085c17e6a77a12cf16055d9e21ac5f5c6c40895 by Waqar Ahmed.
Committed on 11/01/2023 at 17:13.
Pushed by waqar into branch 'master'.

Add Eslint tool

Basic support. You can run the tool manually. In a follow up I will try
to add a "run on save" option that will run the tool automatically for
matching files.

M  +1    -0    addons/project/CMakeLists.txt
M  +15   -10   addons/project/tools/codeanalysisselector.cpp
A  +103  -0    addons/project/tools/eslintcurrent.cpp     [License: LGPL(v2.0+)]
A  +36   -0    addons/project/tools/eslintcurrent.h     [License: LGPL(v2.0+)]

https://invent.kde.org/utilities/kate/commit/7085c17e6a77a12cf16055d9e21ac5f5c6c40895

Git commit a61c29e129b4076810b7aab903229f000e4f0a34 by Waqar Ahmed.
Committed on 11/01/2023 at 17:13.
Pushed by waqar into branch 'master'.

Allow EsLint to run on save

This commit adds api so that a tool can provide a config widget. It also
adds api that allows a tool to provide info on whether it can runOnSave.

M  +16   -0    addons/project/kateprojectcodeanalysistool.h
M  +78   -5    addons/project/kateprojectinfoviewcodeanalysis.cpp
M  +39   -0    addons/project/kateprojectinfoviewcodeanalysis.h
M  +30   -1    addons/project/tools/eslintcurrent.cpp
M  +8    -0    addons/project/tools/eslintcurrent.h

https://invent.kde.org/utilities/kate/commit/a61c29e129b4076810b7aab903229f000e4f0a34

Git commit 1faae7960a58c094057bddd4342fb332781a80cb by Waqar Ahmed.
Committed on 11/01/2023 at 17:13.
Pushed by waqar into branch 'master'.

Support ESLint diagnostic fixes

M  +65   -2    addons/eslint/EslintPlugin.cpp
M  +12   -0    addons/eslint/EslintPlugin.h

https://invent.kde.org/utilities/kate/commit/1faae7960a58c094057bddd4342fb332781a80cb

> really like the idea of a fallback config in future (and maybe even an interface to customize it in the plugin settings); maybe a simple error message would be helpful for now? Something like "No ESLint configuration found

Adding the config can be done if needed. It's a separate plugin so there's no reason it can't have it's own config page. Main problem is that I have little experience in using ESLint and thus I am unaware of what configurations are needed / helpful. Help is definitely welcome :) 

> it would be amazing to be able to have it work live 

This is possible, but only if ESLint is fast enough. With a 3 second delay(current situation), not really. I recently added a formatting plugin, perhaps you have used it already. That plugin also supports prettier and is very fast. To be able to do that, we just run a node instance once and then run prettier inside it and then Kate talks to this instance and does formatting. So far it's working fine. Perhaps such an approach can taken with ESLint as well. 

> I could help write user documentation on the plugin for The Kate Handbook? 

FOR SURE. Help is really needed with our documentation in general. You can even try to improve the plugin on your own and maybe create a patch :)