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.

Sign up for GitHub

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

Jump to bottom

Child content types without "builtInAssociations" considered matching for specific file names #1207

Open
iloveeclipse opened this issue Feb 15, 2024 · 3 comments · May be fixed by #1209
Open

Child content types without "builtInAssociations" considered matching for specific file names #1207

iloveeclipse opened this issue Feb 15, 2024 · 3 comments · May be fixed by #1209
Labels
bug Something isn't working

Comments

@iloveeclipse
Copy link
Member

Follow up on eclipse-tm4e/tm4e#703 (comment).

It looks like we have a strange content type handling that results in eclipse-tm4e/tm4e#703 (as one example).
Here what was found by @sebthom:

Ok, I could reproduce it. As I suspected, the issue is in the Eclipse Platform. When querying content types for a file, eventually ContentTypeCatalog#selectMatchingByName will be invoked.

When this method is called with the file extension "txt" it will return the directly mapped contenttype org.eclipse.core.runtime.text and all child content types that have no "builtInAssociations", e.g. have no file-extensions, file-patterns, file-names defined. So it will return these additional content types if the language pack is installed:

  • org.eclipse.tm4e.language_pack.basetype
  • org.eclipse.tm4e.language_pack.markdown-math
  • org.eclipse.tm4e.language_pack.markdown_latex_combined
  • org.eclipse.tm4e.language_pack.cpp_embedded_latex

I don't think this behavior is correct.

Originally posted by @sebthom in eclipse-tm4e/tm4e#703 (comment)
@iloveeclipse iloveeclipse added the bug Something isn't working label Feb 15, 2024
@iloveeclipse iloveeclipse mentioned this issue Feb 15, 2024
Closed
iloveeclipse added a commit to iloveeclipse/eclipse.platform that referenced this issue Feb 15, 2024
@iloveeclipse
Don't consider "unassociated" child content types while matching files
3cb1453 
Fixes eclipse-platform#1207
@iloveeclipse iloveeclipse linked a pull request Feb 15, 2024 that will close this issue
Draft
@iloveeclipse
Copy link
Member Author

I don't know what kind of use case was behind originally implemented code, may be there is a test that would fail, let see: #1209
Current code comment doesn't make much sense for me...

@iloveeclipse
Copy link
Member Author

There are at least some tests failing, the question is if they are really wanted use that feature or "worked by occasion" because of the current implementation:

Test Result (11 failures )
org.eclipse.core.tests.internal.propertytester.FilePropertyTesterTest.testExistingNsRootElementFile
org.eclipse.core.tests.resources.CharsetTest.testBug62732
org.eclipse.core.tests.resources.ContentDescriptionManagerTest.testNatureContentTypeAssociation
org.eclipse.core.tests.resources.content.IContentTypeManagerTest.testAlias
org.eclipse.core.tests.resources.content.IContentTypeManagerTest.testRegistry
org.eclipse.core.tests.resources.content.IContentTypeManagerTest.testContentAndNameMatching
org.eclipse.core.tests.resources.content.IContentTypeManagerTest.testIOException
org.eclipse.core.tests.resources.content.IContentTypeManagerTest.testRootElementAndDTDDescriber
org.eclipse.core.tests.resources.content.IContentTypeManagerTest.testDoubleAssociation
org.eclipse.core.tests.resources.content.IContentTypeManagerTest.testSignatureBeyondBufferLimit

@sebthom
Copy link

sebthom commented Feb 15, 2024

Some more infos:

  • the current behaviour of ContentTypeCatalog#selectMatchingByName has not been changed in the last 16 years
  • I found this in documentation at help.eclipse.org / Content Types:
    • base-type: the fully qualified identifier of this content type's base type. This content type will inherit its base type's file associations, content describer and default charset, unless they are redefined.
      → I am pretty sure not many people are aware of this and its implications

With the current behaviour if I define a content type like this:

    <content-type
       id="contenttype.example"
       base-type="org.eclipse.core.runtime.text"
       name="Example" 
       file-extensions="example" />

Platform.getContentTypeManager().findContentTypesFor("txt") will not return it.

However if I configure the content type like this which:

    <content-type
       id="contenttype.example"
       base-type="org.eclipse.core.runtime.text"
       name="Example" />
    <file-association
      content-type="contenttype.example"
      file-extensions="example" />

Platform.getContentTypeManager().findContentTypesFor("txt") will also contenttype.example.

I think this strongly violates the Principle of Least Surprise

The big question is how can I define a content type that does not inherit its base type's file assocations without setting new file associations. I.e. if you want an intermediate content-type that has no file associations or where the content type is determined based on the content itself.

The ugly workaround at the moment is

    <content-type
       id="contenttype.example"
       base-type="org.eclipse.core.runtime.text"
       name="Example" 
       file-extensions="PUT_SOME_VALUE_THAT_HOPEFULLY_NO_ONE_ELSE_USES" />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't consider "unassociated" child content types while matching files iloveeclipse/eclipse.platform
2 participants
@sebthom @iloveeclipse