Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 364481 - Associate editor with content types
Summary: Associate editor with content types
Status: RESOLVED FIXED
Alias: None
Product: Orion
Classification: ECD
Component: Client (show other bugs)
Version: 0.3   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 0.4 M1   Edit
Assignee: Mark Macdonald CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 349802 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-11-22 10:39 EST by John Arthorne CLA
Modified: 2011-12-07 16:42 EST (History)
5 users (show)

See Also:


Attachments
Screen shot (4.75 KB, image/png)
2011-11-22 10:40 EST, John Arthorne CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Arthorne CLA 2011-11-22 10:39:09 EST
I20111121

1) Install code mirror modes plugin:

http://mamacdon.github.com/orion-codemirror/codeMirrorPlugin.html 

2) Create a file called test.php with the following contents:

<html>
 <body>
 <p>
 Hello and goodbye.
 </p>
 </body>
</html>

-> The word "and" is colored in red. This is wrong because it is not enclosed in a script tag so it should be like plain html. Also there is no syntax coloring of tags.
Comment 1 John Arthorne CLA 2011-11-22 10:40:34 EST
Created attachment 207370 [details]
Screen shot
Comment 2 Mark Macdonald CLA 2011-11-22 11:02:08 EST
CodeMirror's php mode uses different MIME types to distinguish PHP in html (application/x-httpd-php) from a plain, unwrapped PHP script (text/x-php).

In the Orion plugin, I've mapped .phtml to "application/x-httpd-php" and and .php to "text/x-php". (Note that if you recreate your example as test.phtml, the HTML tags are highlighted properly.)
Comment 3 Mark Macdonald CLA 2011-11-22 11:05:09 EST
I think there's a general issue here... Orion's reliance on file extension is often inadequate. CodeMirror's MIME-type approach seems better, but we'd still need a way for a user to specify that (for example) a .pref file should be recognized as "application/json".
Comment 4 Ken Walker CLA 2011-11-22 11:43:57 EST
Are mime types not the way to go?  File types seem inadequate to provide the information you need for the actual file and the user might want to remap these anyway.
Comment 5 Mark Macdonald CLA 2011-11-23 16:54:28 EST
How about something like this. Here's an example with mocked-up extensions.
---------------------------------------------------------
Declaring an "HTML" type:

  registerServiceProvider("orion.file.contentType", {}, {
    id: "text.html"
    extends: "text.plain" 
    name: "HTML"
    mime: ["text/xml", "application/xhtml+xml"]
    filename: []
    extension: ["html", "htm"] });

Associating "HTML" with the Orion editor:

  registerServiceProvider("orion.navigate.openWith", {}, {
    contentType: "text.html"
    editor: "orion.web.editor" });

Declaring the Orion editor:

  registerServiceProvider("orion.edit.editor", {}, {
    id: "orion.web.editor"
    href: "../edit/edit.html#${Location}" });
---------------------------------------------------------

User prefs might override the stuff found in orion.file.contentType and orion.navigate.openWith if you've made customizations.
Comment 6 Mark Macdonald CLA 2011-11-29 12:45:19 EST
I have a first crack at this working. I am using contentTypes for editor association and lookup.

I removed the MIME types: they were purely descriptive, and I couldn't think of a use case where they added anything. Simon also pointed out that MIMEs can be unreliable because of misconfigured servers, etc. People who care, like the CodeMirror plugin, can do their own contentType to MIME mapping.

One question I haven't worked out is how contentTypes relate to the "orion.navigate.command" extension.
It seems more natural for file-related commands to target a content type rather than pattern-match on the filename. Maybe I could expose a file's content type as a synthetic property that you can validate a command against? You could then do something like this:
  validationProperties: {
    ContentType: "text.xml"
  }
Susan, any opinion on that?
Comment 7 Susan McCourt CLA 2011-11-30 12:11:40 EST
> One question I haven't worked out is how contentTypes relate to the
> "orion.navigate.command" extension.
> It seems more natural for file-related commands to target a content type rather
> than pattern-match on the filename. Maybe I could expose a file's content type
> as a synthetic property that you can validate a command against? You could then
> do something like this:
>   validationProperties: {
>     ContentType: "text.xml"
>   }
> Susan, any opinion on that?

Yes, I think we would move any extension that currently validates on file extension to use the content types.  But we probably want to support an array of content types vs. assuming just one.

The question is whether we want to use backward compatibility code to make the old plugins still work, and who would be responsible for that.  

Note we have a request to implement the same kind of validation in orion.edit.command (but we wouldn't have to worry about backward compatibility here).  See bug 337766 comment 4.
Comment 8 Susan McCourt CLA 2011-11-30 12:16:45 EST
> The question is whether we want to use backward compatibility code to make the
> old plugins still work, and who would be responsible for that.  

I don't mean who will write the code, but who (what object) is responsible...would we just parse the file extensions in the code that currently reads orion.navigate.command or do we want a common utility that validates a file against a validation expression, and would also check extensions if a content type was not specified...
Comment 9 Mark Macdonald CLA 2011-12-05 13:09:08 EST
(In reply to comment #7)
> But we probably want to support an array of content types vs. assuming just one.

Yeah, I agree.

> The question is whether we want to use backward compatibility code to make the
> old plugins still work, and who would be responsible for that.

I wasn't envisioning content types totally replacing validation properties, just enhancing them. We could still allow matching on "Name" (or any other file property) like plugins are doing now, it just wouldn't be the recommended way to scope your contribution to a file type.

> Note we have a request to implement the same kind of validation in
> orion.edit.command (but we wouldn't have to worry about backward compatibility
> here).  See bug 337766 comment 4.

(In reply to comment #8)
> I don't mean who will write the code, but who (what object) is
> responsible...would we just parse the file extensions in the code that
> currently reads orion.navigate.command or do we want a common utility that
> validates a file against a validation expression, and would also check
> extensions if a content type was not specified...

Makes sense to move the validation stuff out of fileCommands into a common util that is content-type-aware. Then we can reuse it from whatever extension handlers (navigate.command, edit.command, etc) need it.
Comment 10 Mark Macdonald CLA 2011-12-05 14:06:41 EST
Opened Bug 365649 for getting content type supported in the orion.navigate.command extension.

Marking this bug as fixed; see http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=771d55acdcda294a9f4da25e40f879829671161c
Comment 11 Mark Macdonald CLA 2011-12-05 14:09:53 EST
*** Bug 349802 has been marked as a duplicate of this bug. ***
Comment 12 John Arthorne CLA 2011-12-07 16:03:11 EST
Changing title because the work that was done had nothing to do with code mirror and PHP inside HTML.
Comment 13 Mark Macdonald CLA 2011-12-07 16:12:43 EST
(In reply to comment #12)

Yeah, I hijacked this bug for other purposes, sorry.

The original complaint is basically that there's no way to take a .php file and open it like a .phtml file. I think we could fix this an action like "Open as [whatever]" which would let you treat some random file as a type that Orion already knows about.
Comment 14 Mark Macdonald CLA 2011-12-07 16:42:01 EST
(In reply to comment #13)
> The original complaint is basically that there's no way to take a .php file and
> open it like a .phtml file. I think we could fix this an action like "Open as
> [whatever]" which would let you treat some random file as a type that Orion
> already knows about.

Opened bug 365967 for this.