Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

problems-view: problems order changes on preference changes #8983

Closed
DucNgn opened this issue Jan 22, 2021 · 8 comments · Fixed by #9691
Closed

problems-view: problems order changes on preference changes #8983

DucNgn opened this issue Jan 22, 2021 · 8 comments · Fixed by #9691
Assignees
Labels
bug bugs found in the application problems issues related to the problems widget

Comments

@DucNgn
Copy link
Contributor

DucNgn commented Jan 22, 2021

Bug Description:

  • The order of displaying problems is switched on any preferences changes.
  • After a preference change, it seems that the file nodes with warnings are prioritized (need investigation)

Steps to Reproduce:

  1. Download the test workspace and open the workspace in theia
  2. Open files with errors, then open files with warnings (order matters, file with warnings should not be the very first one to be opened).
    • ie: open a (error), open b (error), open package.json (warning).
  3. Open problems-view by calling Toggle Problems View from the command palette.
  4. Observe problems in the view are displayed chronologically with respect to the timeline that files were opened.
  5. Open preferences, change any settings. Observe the order of problems changes automatically in the view.

problems-view-issue

Additional Information

  • Operating System: confirmed on macOS, Windows, Linux
  • Theia Version: Master 1.9.0
@paul-marechal
Copy link
Member

I am unable to reproduce on Windows. Although something seems to happen with the problem view when changing preferences, the order is not randomized for me using master...

@DucNgn
Copy link
Contributor Author

DucNgn commented Jan 23, 2021

@marechal-p I updated the issue description with more info and included my test workspace. Apparently, the order in which you open files matters. Can you try to reproduce again?

@vince-fugnitto vince-fugnitto added bug bugs found in the application problems issues related to the problems widget labels Jan 23, 2021
@JonasHelming
Copy link
Contributor

@dukengn : I debugged this a bit. First of all, the current implementation of the marker data model does not guarantee any specific order, neither by URI, not by owner (owner = source, e.g. ESlint). The order "by chance" is the order in which things get added, but again, as everything is stored in maps it is not guaranteed.
The reason why the order actually changes is that the TypeScript owner (for the "cannot find name" error) does completely clear its markers on a settings change. Therefore, it is added at the end of the order of owners when it adds the markers again. It even clears and re adds the markers three times in your example. Other owners (e.g. ESlint) do only a set, which does not remove the entry from the map. The interesting questions:

  1. Why does the TypeScript owner (language server) clear and add
  2. Why does it do this three times
  3. and 2. might be either in the TypeScript server or the change notification that the Theia VS Code API triggers.
  4. What is the expected behavior, should we generally sort the problems view and if so how?

Could you maybe try to reproduce this in VS Code? This would give a hint whether the TypeScript server behaves weird here. Also, w could probably just implement a similar sorting behavior if VS Code has any...

@JonasHelming JonasHelming self-assigned this Mar 15, 2021
@vince-fugnitto
Copy link
Member

Also, w could probably just implement a similar sorting behavior if VS Code has any...

@JonasHelming from what I understand vscode has a consistent sorting behavior (by resource):

@JonasHelming
Copy link
Contributor

@vince-fugnitto : Thanks, I guess we should do the same, seems reasonable. Do you know how they sort within one URI, i.e. by owner?

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Mar 15, 2021

@vince-fugnitto : Thanks, I guess we should do the same, seems reasonable. Do you know how they sort within one URI, i.e. by owner?

@JonasHelming I've already added the sorting within a URI (it prioritizes severity then line and column number):

Their respective test-cases for sorting markers:

@JonasHelming
Copy link
Contributor

VS Code actually sorts the first level of the tree by

  1. The highest severity occurring in a node
  2. The path

results are cached though, you need to reopen the view so see a sort change on the highest level.

Do we want to do the same? @vince-fugnitto

@vince-fugnitto
Copy link
Member

Do we want to do the same? @vince-fugnitto

Ideally we should do the same if we believe it produces the best results which I think is true.
The initial sorting I added was only in respect for a given resource (uri).

LukasBoll added a commit to LukasBoll/theia that referenced this issue Jul 1, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Jul 6, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Jul 7, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Jul 29, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Aug 18, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Sep 20, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Sep 22, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Sep 25, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
LukasBoll added a commit to LukasBoll/theia that referenced this issue Oct 4, 2021
Fixes eclipse-theia#8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
msujew pushed a commit that referenced this issue Oct 20, 2021
Fixes #8983

Previously info-marker-nodes were ordered by the time they have been reported.
This commit will sort the info-marker-nodes based on the following rules:
- nodes will be sorted based on the most severe marker they contain
- if nodes contain equally severe markers, they are sorted by the URI

Signed-off-by: Lukas Boll <lukas-bool@web.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application problems issues related to the problems widget
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants