[GitHub] [netbeans] jtulach commented on a change in pull request #2797: [NETBEANS-5471] Add MIMEResolver annotation using regex

[GitBox <gi...@apache.org>]

jtulach commented on a change in pull request #2797:
URL: https://github.com/apache/netbeans/pull/2797#discussion_r592928750



##########
File path: platform/openide.filesystems/src/org/openide/filesystems/MIMEResolver.java
##########
@@ -277,6 +286,44 @@ protected final boolean isUserDefined(FileObject mimeResolverFO) {
         public String[] doctypePublicId() default {};
     }
     
+    /**
+     * Often a mime type can be deduced just by looking at a file name. If that is
+     * your case, this annotation is for you. It associates a file name with
+     * provided mime type.

Review comment:
       Missing `@since`, otherwise the change seems OK.

##########
File path: platform/openide.filesystems/src/org/netbeans/modules/openide/filesystems/declmime/FileElement.java
##########
@@ -205,6 +221,18 @@ private void readExternal(DataInput in) throws IOException {
                     names.add(new FileName(in));
                 }
             }
+            int fileNamePatternsSize = -1;
+            try {
+                fileNamePatternsSize = in.readInt();

Review comment:
       I'd like to see a test covering this part - e.g. to read old format as well as new format.

##########
File path: platform/openide.filesystems/src/org/openide/filesystems/MIMEResolver.java
##########
@@ -179,6 +180,10 @@ protected final boolean isUserDefined(FileObject mimeResolverFO) {
         /** Display name to present this type of objects to the user.
          */
         public String displayName();
+        /** Display filter to present this type of objects to the user, such as
+         * {@literal "*.html"}.
+         */

Review comment:
       Missing `@since` tag.

##########
File path: platform/openide.loaders/src/org/openide/loaders/DataNode.java
##########
@@ -611,6 +614,60 @@ public Long getValue() {
         
     }
     
+    private final class MIMETypeProperty extends PropertySupport.ReadOnly<String> {
+
+        public MIMETypeProperty() {
+            super("mimeType", String.class, DataObject.getString("PROP_mimeType"), DataObject.getString("HINT_mimeType"));
+        }
+
+        public String getValue() {
+            return getDataObject().getPrimaryFile().getMIMEType();
+        }
+
+    }
+    
+    private String getMIMEResolverAttribute(FileObject primaryFile, String attributeName) {
+        final String mimeType = primaryFile.getMIMEType();
+        if (mimeType != null) {
+            FileObject mimeResolversFolder = FileUtil.getConfigFile("Services/MIMEResolver"); // NOI18N
+            if (mimeResolversFolder != null) {

Review comment:
       Isn't this a bit time consuming to iterate all resolvers for every `DataNode` displayed? 
   
   I am not sure what this method is good for, but it doesn't feel right.

##########
File path: platform/openide.loaders/src/org/openide/loaders/DataNode.java
##########
@@ -540,6 +540,9 @@ protected Sheet createSheet () {
                 ss.put(new ExtensionProperty());
             }
             ss.put(new SizeProperty());
+            ss.put(new MIMETypeProperty());

Review comment:
       Deserves a note in `apichanges.xml` of `openide.loaders`.
   
   Wouldn't it be (from a usability standpoint) better to have just a single property (mime type) with custom property editor to show the details? Three properties for MIME type, it's display name and a filter pattern seem a bit too overkill for majority of users.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists