Bug 456493 - Feature Request: ESLint
Summary: Feature Request: ESLint
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR wishlist
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-08 21:17 UTC by nathan
Modified: 2023-01-21 20:42 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
ESLint Error Duplication in Diagnostics Output (3.18 MB, video/webm)
2023-01-08 05:22 UTC, nathan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description nathan 2022-07-08 21:17:25 UTC
It would be great if Kate had ESLint integration like many other text editors. I'm trying to get away from electron applications; I'm quite enjoying using Kate for webdev, and I find that I don't need many of the VSCode plugins I'd installed over time, but ESLint really is quite important.
Comment 1 Waqar Ahmed 2022-07-21 17:12:03 UTC
If someone is up for it, this should be easy to implement.
Comment 2 nathan 2023-01-04 01:34:31 UTC
(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.
Comment 3 Waqar Ahmed 2023-01-04 06:00:31 UTC
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
Comment 4 nathan 2023-01-07 01:53:08 UTC
(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?
Comment 5 Waqar Ahmed 2023-01-07 10:12:30 UTC
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
Comment 6 nathan 2023-01-08 05:21:42 UTC
(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.
Comment 7 nathan 2023-01-08 05:22:17 UTC
Created attachment 155100 [details]
ESLint Error Duplication in Diagnostics Output
Comment 8 Waqar Ahmed 2023-01-08 11:36:06 UTC
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.
Comment 9 nathan 2023-01-08 23:14:58 UTC
(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.
Comment 10 Waqar Ahmed 2023-01-09 07:27:40 UTC
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.
Comment 11 nathan 2023-01-09 23:48:08 UTC
(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?
Comment 12 Waqar Ahmed 2023-01-10 05:05:41 UTC
> 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.
Comment 13 nathan 2023-01-10 23:11:34 UTC
(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?
Comment 14 Waqar Ahmed 2023-01-11 18:10:42 UTC
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
Comment 15 Waqar Ahmed 2023-01-11 18:10:58 UTC
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
Comment 16 Waqar Ahmed 2023-01-11 18:11:07 UTC
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
Comment 17 Waqar Ahmed 2023-01-21 20:42:30 UTC
> 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 :)