-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Added pmarker sorting by owner #9211
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JonasHelming thank you for taking the time to improve the sorting. I only have minor comments about formatting and cleanup. Do you also mind updating the commit, we never referred to problem markers as pmarker
and it may be confusing to others.
Suggestion:
problems: add fallback marker sort based on 'owner'
Fixes: #6572
The following commit adds an additional problem marker sort based on 'owner` as a fallback to 'severity', 'line number' and 'column number'.
packages/markers/src/browser/problem/problem-tree-model.spec.ts
Outdated
Show resolved
Hide resolved
packages/markers/src/browser/problem/problem-tree-model.spec.ts
Outdated
Show resolved
Hide resolved
packages/markers/src/browser/problem/problem-tree-model.spec.ts
Outdated
Show resolved
Hide resolved
@vince-fugnitto : Thank you for the review, I took all suggestions! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have minor comments, I was unable to produce the secondary owner
marker with the example project you provided, what did you do to turn on 'Parsing error has been set for....' in your .gif?
packages/markers/src/browser/problem/problem-tree-model.spec.ts
Outdated
Show resolved
Hide resolved
packages/markers/src/browser/problem/problem-tree-model.spec.ts
Outdated
Show resolved
Hide resolved
packages/markers/src/browser/problem/problem-tree-model.spec.ts
Outdated
Show resolved
Hide resolved
Fixes: eclipse-theia#6572 The following commit adds an additional problem marker sort based on 'owner` as a fallback to 'severity', 'line number' and 'column number'.Added pmarker sorting by owner Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
done |
I started a Gitpod workspace with the Eclipse Theia code base and launched the example product from there which accesses the example problems workspace. |
All comments should be addressed, thanks for the review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you 👍
fixed #6572
Signed-off-by: Jonas Helming jhelming@eclipsesource.com
What it does
adds sorting by owner to problem markers (after severity, line and column
How to test
See also here: #6572 (comment)
Review checklist
Reminder for reviewers