[Feature] indicate visually folders/files containing errors

[yyoncho]

This is a common feature for most of the IDEs(e. g. Eclipse, vscode) in which folders containing errors are in red/yellow.

1. Expose function accepting a list of file->status. Each of the files could be in state error/warning/normal and based on that treemacs will update the color of each of the file parents.
2. Integration with flycheck?

[Alexander-Miller]

Integration with flycheck?

Either that or flymake. How else is treemacs supposed to know what to do?

I've looked around flycheck and it looks like it would require work from both sides. As of now flycheck seems to provide a list of errors for the current buffer only. For example in this setup I make an error in y.cpp and include it in x.cpp. The hooks offered by flycheck give me acces only to the errors in x.cpp:

(#s(flycheck-error x.cpp c/c++-clang /home/a/Documents/git/FO/x.cpp 2 nil In include /home/a/Documents/git/FO/y.cpp error nil nil)
 #s(flycheck-error x.cpp c/c++-clang /home/a/Documents/git/FO/x.cpp 4 1 expected unqualified-id error nil nil))

y.cpp has it own errors when I run the checker in its buffer:

(#s(flycheck-error y.cpp c/c++-clang /home/a/Documents/git/FO/y.cpp 2 1 unknown type name 'main' error nil nil) 
 #s(flycheck-error y.cpp c/c++-clang /home/a/Documents/git/FO/y.cpp 2 5 expected unqualified-id error nil nil))

Let's ask the other side then.

Hello @cpitclaudel, would you be interested in a bit of jolly cooperation? Ideally flycheck just needs to expose a hook where I can access all the errors/warnings etc. it has found, without being trimmed for the current buffer.

[cpitclaudel]

Hi @Alexander-Miller, thanks for the ping :) You might be interested in https://github.com/flycheck/flycheck/pull/1427 for some background reading. I'll summarize here though.

The basic issue is that Flycheck doesn't typically trim a list of errors to keep only the ones from the current buffer; instead, it asks checkers about the current file, and the checker typically returns errors for that file only.

There are a few checkers that do not operate on a single file, though, and instead show errors for the complete project (Elm, rust, and Haskell work like this, I think). For these, Flycheck did use to trim errors, but it doesn't anymore (instead, we recently started reporting errors from files other than the current one as coming from the first line of the file).

Hope this helps :) (cc @fmdkdd)

[yyoncho]

@Alexander-Miller

Either that or flymake. How else is treemacs supposed to know what to do?

I believe that making the treemacs open for integration with arbitrary error provider via method similar to the one described in 1) will be a good solution. E. g. in lsp-mode there is hook for diagnostics reported and it will be trivial to make the integration if there is such method. Although the integration could go though flycheck.

Offtopic: treemacs rocks! Today I tried multiroot and it is great! This package will convert a lot of people to emacs users :) .

[Alexander-Miller]

Hope this helps :)

It does, thanks. Just more question: is there a straightforward way to change the command used by a checker, or do I need to define my own custom checker? I'm using a makefile for treemacs that can compile the entire project. Telling the elisp checker to run make compile for some specific buffers might be useful, if only for documentation and testing purposes.

E. g. in lsp-mode there is hook for diagnostics reported and it will be trivial to make the integration if there is such method

Ah, I had completely forgotten about lsp. I thought flycheck and flymake were the only linting frameworks for emacs. I am going to need a proper api on my side regardless of integrates with what. That'll be the easy part anyway. The hard part is not showing some warning icon next to /src/foo.el after a build has run, but knowing when you have to display it when you expand /src. I'll need to do some non trivial internal expansions to make that possible.

There also needs to be a solid lint status invalidation story behind this. The last thing you want is to see warnings and errors that are no longer uptodate and that you cannot get rid of.

All in all this issue is looking to become the Next Big Thing. Which is why I'll put it towards the bottom of my todo list for now. First I'd like to get some minor tasks done, reduce the issue count and finish with those pull requests.

[cpitclaudel]

Just more question: is there a straightforward way to change the command used by a checker

Yup, the (flycheck-checker-get checker 'command) is setf-able.

[yyoncho]

I was recently thinking about this issue and the functionality that treemacs could provide and I realised that showing errors is only subset of the problems that it can solve. Here are few more examples from lsp-mode standpoint of view:

1. Use different icons sources folders or (java) packages
2. Mark methods that are unit tests
3. After execution of test suite mark tests in green/red/in-progress

All of these could be handled if there is a generic treemacs function set-node-style (+ keymap?) which will accept path to the node e. g.

;; oversimplified example
(treemacs-set-node-style "~/src/ClassName.java#TestClass$testMethodName", {unitTestIcon} {unitTestFont} {keymap} ... )

What do you think? PS: I have long TODO list in lsp-mode/lsp-java, at some time I could join the effort.

[Alexander-Miller]

What do you think?

The general idea is solid and I think the correct path forward, but all these new features require setting up hooks and interventions somewhere very deep in the ~Heart of Darkness~ render logic, and being paranoid about performance I think it's quite terrifying.

If we wanted to, for example, mark some file in green

[Alexander-Miller]

Bad github, I wasn't done yet!

To continue my thoughts from above:

If we wanted to, for example, mark /src/FooTest.java in green because it passed all unit tests then in the worst case treemacs would need to compare the path of every single file it renders whether it is /src/FooTest.java (or one of the other 20 files whose display we've changed for other reasons).

So there needs to be some good render hook infrastructure, as local to the parts it changes as possible, and with as many of its parts doing their work in O(1) or with a small n as possible.

Tags need to be looked at separately as well. Right now treemacs identifies a tag by what it calls a "tag path". It's basically a list on how to get to the tag, with the tag itself at the front, for example ("int length()" "/src/String.java" "Classes" "String"). That's about the best I do with the info from imenu.

What do the tags look like that come from lsp? Is there a different way to uniquely identify a specific tag that can be used as a primary key? That kind of info is used in treemacs for example to remember that you've expanded src/String.java -> Classes -> String and then to reopen the Classes section when you close and reopen String.java. I would also need this kind of identifier to mark a passed unit test green.

[yyoncho]

What do the tags look like that come from lsp?

LSP mode is integrated with imenu (via imenu-create-index-function) so as long as you generate the tags using imenu as a source lsp will be able to match them. Here it is the spec for document symbol which corresponds serves as a backend of imenu if you need more info: https://microsoft.github.io/language-server-protocol/specification#textDocument_documentSymbol

Is there a different way to uniquely identify a specific tag that can be used as a primary key?

As far as I can see there is a marker/point in the imenu elements so it can be used to uniquely identify the node.

[Alexander-Miller]

As far as I can see there is a marker/point in the imenu elements so it can be used to uniquely identify the node.

Ah, that's definitely an interesting option, I hadn't considered that. It might make things easier for me, but the whole tag path thing still needs to be kept around, container nodes like "Classes" don't really have a position.

And on a positive node, it is possible to combine faces right as the icons are being rendered, as you can see here with the underline face being spliced in. That will make much simpler (and faster!) than mucking about with overlays.

[Compro-Prasad]

Now that lsp support is properly implemented in Emacs, do you think it would be better to look into lsp-mode?

[Alexander-Miller]

Assuming lsp does have advantages over flycheck, like knowing the states of all files in the project, except just the current one. It's certainly worth looking into.

It'll try and put up a simple prototype soon enough, but I'd prefer to get some mileage out of https://github.com/Alexander-Miller/treemacs/issues/226 before beginning this in earnest.

[yyoncho]

@Compro-Prasad I am two issues away from starting a project which will do integration between lsp-mode and treemacs, including:

1. Sync between lsp-mode workspace folders and treemacs projects.
2. Implementation of some of the lsp-mode functions like errors, references, document symbols using treemacs extension API.
3. Display the errors in the treemacs tree once this PR is in.

[Alexander-Miller]

@yyoncho I've built a small prototype now that looks like this: https://imgur.com/ngTLl7h. I am now wondering what the api should look like in detail. Using the diagnostics in treemacs makes little sense outside of lsp projects (with a few flycheck based exceptions), so I think the api should be designed based on lsp-mode's data structures. Specifically I have 2 goals:

1. Using it should be as straightforward for you as possible. You should not need complex transformations or have to use different mappings in your data, both to avoid likely bugs and help performance.
2. The granularity should be right. I assume lsp delivers the diagnostic data for an entire project, or would you want to be able to set the state of the files in just a specific directory?

And some other questions:

AFAIU lsp-mode may receive new diagnostic data in basically every key event. Does that mean you'll make just as many requests to treemacs, or is there some kind of buffering (debouncing things on side of treemacs would be no problem)?

An annoying problem I faced in git-mode is having a file like /src/main/foo.el be highlighted mean that I also need to highlight /src/main and /src as modified. Same would apply for diagnostics. Are language servers able to supply directories' cumulated diagnostic states or do we have to make that happen on our end?

[yyoncho]

2. The granularity should be right. I assume lsp delivers the diagnostic data for an entire project, or would you want to be able to set the state of the files in just a specific directory?

We get the diagnostics file by file.

AFAIU lsp-mode may receive new diagnostic data in basically every key event. Does that mean you'll make just as many requests to treemacs, or is there some kind of buffering (debouncing things on side of treemacs would be no problem)?

We definitely will need debouncing. IMO lsp-mode part is relatively trivial so I guess we should focus on creating a design which will yield optimal performance on treemacs side.

An annoying problem I faced in git-mode is having a file like /src/main/foo.el be highlighted mean that I also need to highlight /src/main and /src as modified. Same would apply for diagnostics. Are language servers able to supply directories' cumulated diagnostic states or do we have to make that happen on our end?

We should do that on our side.

[Alexander-Miller]

I have found a trouble spot that we need to get around: I need a persistent associative store for the diagnostics data so it won't disappear when you refresh a project and will be displayed when you expand a new directory, and the old state can be removed when apply a new one. I assume it will be no problem to also supply the treemacs project's root when applying diagnostics?

so I guess we should focus on creating a design which will yield optimal performance on treemacs side.

The example you saw in the screenshot has a runtime (uncompiled) of 1ms. The investment into a solid dom implementation has payed off.

[yyoncho]

We already store the errors and they are accessible via lsp-diagnostics.

[Alexander-Miller]

Directly calling that would mean a hard dependency and I'd rather avoid that. I'll have a look at what that api looks like, writing a generic diagnostics-provider-function should not be a problem.

[yyoncho]

Yes. We already have a project to host the integration - lsp-treemacs.

[Alexander-Miller]

Ok, instead of talking about things in the abstract I've pushed a preview version in https://github.com/Alexander-Miller/treemacs/commit/fddb401c1a5fce6e6fd320b6a2948f5f387d3c08 that you can use to get a feeling for how it works.

You need to require it manually and there is basically zero plumbing so the overlays won't survive a refresh etc, but core display application should work as advertised.

[Alexander-Miller]

Added debouncing as well, things should now be ready for proper beta testing.

[yyoncho]

Thank you, I will test it and let you know. FYI now I have ported all of the tree components in dap-mode to use treemacs.

[yyoncho]

I am having some troubles using the API: is this the corrent way:

(treemacs-apply-diagnostics "/home/kyoncho/Sources/lsp/dap-mode/features/fixtures/test-project/src/test/java/temp/AppTest.java" 
  'treemacs-diagnostic-warning)

(treemacs-apply-diagnostics "/home/kyoncho/Sources/lsp/dap-mode/features/fixtures/test-project/" 
  'treemacs-diagnostic-warning)

[Alexander-Miller]

The first argument should be the treemacs project root, the &rest is a list of file face pairs (I can change it to a single argument if that is more convenient): https://imgur.com/a/iqGGWRt. I need to store the diagnostic information in a way that makes it easy to look up when some node is expanded and easy to reset when new info arrives.

At least I think that's what I need to do. Based on the lsp-treemacs code I have seen you query for the diagnostic state of all the servers and (lsp-)workspaces at once, right? If that is the case I can just use a simple stupid global cache and worry about a project-based solution for something like flycheck later.

And while we are already on the topic: there's also a small matter of performance: The code is debounced on the side of treemacs, but you still have to transform and collect the data from lsp-mode's struct types whenever it arrives, and I think this part can be easily optimized too.

Instead of always actively transforming the data for treemacs we can make the process lazy by passing a lambda to treemacs that will only be called up to perform the transformations when the debouncing gives it a go (right now that at most once every 3 seconds). What do you think?

[yyoncho]

I got some troubles running the sample:

(-each (overlays-in start end) #'delete-overlay)

This was failing for end being nil.

Instead of always actively transforming the data for treemacs we can make the process lazy by passing a lambda to treemacs that will only be called up to perform the transformations when the debouncing gives it a go (right now that at most once every 3 seconds). What do you think?

Make sense.

[Alexander-Miller]

This was failing for end being nil.

I'm busy at work for the next few days, so I expect I'll get around to fixing this some time next week. In the meantime you can just locally change the start & end to be point-min & point-max.

[yyoncho]

I will start working on this feature for the next few days. I wonder whether the code could be made more generic and let writing extension code which can apply overlay on any treemacs node. Example usecase is to mark Root projects that are also lsp-mode roots.

[Alexander-Miller]

Actually that's already possible: https://imgur.com/a/QAHYNTM

The repetition of the root is ugly of course, but rewriting that part is in the pipeline anyway.

[Alexander-Miller]

In case you haven't pulled: I had managed to push the discussed changes before the flu fully got me. It's all simplified and lazyfied now.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Alexander-Miller]

Stayin alive.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Alexander-Miller]

Stayin alive. An api for this is mostly done in https://github.com/Alexander-Miller/treemacs/pull/866

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Alexander-Miller]

Stayin alive.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[aspiers]

Still interested in this feature.

[cpitclaudel]

Is there a way to unsubscribe from all stale bot notifications? https://drewdevault.com/2021/10/26/stalebot.html

[aspiers]

@cpitclaudel commented on April 11, 2022 4:59 PM:

Is there a way to unsubscribe from all stale bot notifications? drewdevault.com/2021/10/26/stalebot.html

Thank you for sharing, this is a great article which exactly captures my own thoughts on stale bots.

[Alexander-Miller]

Is there a way to unsubscribe from all stale bot notifications?

I don't think you can do anything other than filter "stale-bot" in your mail client.

All I want from the bot is a regular reminder anyway, so I've also disabled its auto-closing feature now. All you'll get now is a bump after 60 days of inactivity.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity.

[ferranmarlet]

Is this still under development? BTW treemacs is great, thank you for your great work!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity.

[AryanPandeyy]

Looking forward for this feature.

[Alexander-Miller]

Is this still under development?

If it's not closed it means it's somewhere in the backlog.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity (this bot only works as a reminder, it will not close issues).