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

Element order in project explorer arbitrarily changes if label decoration is involved #716

Closed
travkin79 opened this issue Feb 23, 2024 · 5 comments · Fixed by #717
Closed

Comments

@travkin79
Copy link
Contributor

travkin79 commented Feb 23, 2024

The observed issue
The order of elements in the project explorer differs from time to time, for example, if we have a folder under version control (git) and some of the contained files are modified, but not committed. The order should not change arbitrarily.

Steps to reproduce

  1. Switch to the C++ perspective
  2. Take any C++ project under version control (e.g. git) with empty git status (no uncommitted changes)
  3. Open any folder in that project in the project explorer (list the children)
  4. Modify any file in that folder, leave the other files unchanged
  5. Select the folder in the project explorer
  6. Hit F5 key multiple times, with only short delays of some ms (triggers refresh operation)
  7. See how the modified files sometimes are list first (because of the "> " prefix) and sometimes are listed in their natural order (ordered by file name)

Expected behavior
The files should always stay ordered using the file name, even if they are decorated with e.g. a "> " prefix.

Screenshots
See an example of differing element order in the project explorer:

Order 1:
image

Order 2:
image

Version Information

  • OS: RHEL 7
  • Custom-build Eclipse IDE version 4.30
  • CDT version 11.4.0

Additional context
I narrowed the issue down to org.eclipse.cdt.ui.CElementSorter#compareWithLabelProvider(Viewer, Object, Object). Here, for elements from categories resources, resource folders, storage, and others the label provider is used to calculate the file's labels for comparison. The decoration is done asynchronously. If there is no cached decoration available, no decoration is used, but the decoration calculation is triggered. I think, that's why the labels returned by the label provider for the modified files sometimes come with the prefix "> " and sometimes without that prefix. That difference leads to differing order in the project explorer.

I took a look at JDT, i.e. org.eclipse.jdt.ui.JavaElementComparator#compare(Viewer, Object, Object). Here, instead of using the label providers an label is calculated without decorating the IResource's name, see org.eclipse.cdt.ui.CElementSorter#getNonJavaElementLabel(Viewer, Object). I gues, it's a good idea to do the same in CElementSorter.

@jonahgraham
Copy link
Member

Your explanation is reasonable and I would happily review a patch. However I think this is yet another case of where something from CDT started out as a copy of the JDT item (i.e. JavaElementSorter -> CElementSorter) but then didn't keep up with the changes. The important change that was "missed" was the JDT change of JavaElementSorter -> JavaElementComparator in eclipse-jdt/eclipse.jdt.ui@7e0c7e7 - can/should we make the same change - i.e. CElementSorter -> CElementComparator?

@travkin79
Copy link
Contributor Author

Thank you for the hint, @jonahgraham. You might be right. That's quite a big commit. I have to check that.

@travkin79
Copy link
Contributor Author

It seems, in eclipse-jdt/eclipse.jdt.ui@7e0c7e7 all occurrences of the obsolete JavaElementSorter (ViewSorter sub-class, ViewSorter is deprecated, too) are replaced with JavaElementComparator (ViewComparator). The main reason seems to be the removal of the org.eclipse.jface.viewers.ViewertSorter#getCollator() method from API. Since the ViewSorter subclasses ViewerComparator that seems to be just a kind of API clean-up.

Though, adopting that change in CDT likely requires quite many places to be changed. I suggest to create a separate issue / PR for that. Besides, I don't think, I could do that change shortly, since I'll be gone for one month beginning on 1. March.

@jonahgraham
Copy link
Member

In that case I will review #716 now and see if we can get that in for 2024-03 (CDT 11.5).

@jonahgraham
Copy link
Member

Thanks @travkin79 for the fix.

If you ever have other issues with the order and have time to do the CElementSorter -> CElementComparator change that would be great. Don't bother creating an issue unless there is an end user visible problem or a plan to fix it. (That is because if we created issues for all unplanned code cleanups with no user visible effect our issue tracker would sadly overflow)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants