After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 598579 - Syntax highlighting wrong for .m files
Syntax highlighting wrong for .m files
Status: RESOLVED FIXED
Product: gtksourceview
Classification: Platform
Component: General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GTK Sourceview maintainers
GTK Sourceview maintainers
Depends on:
Blocks:
 
 
Reported: 2009-10-15 16:05 UTC by Jamie Lavigne
Modified: 2009-10-18 13:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
A sample MATLAB script (74 bytes, text/plain)
2009-10-16 15:32 UTC, Jamie Lavigne
  Details
glob vs mime type (4.79 KB, patch)
2009-10-16 23:08 UTC, Ignacio Casal Quinteiro (nacho)
needs-work Details | Review
glob vs mime type v2 (5.66 KB, patch)
2009-10-17 23:19 UTC, Ignacio Casal Quinteiro (nacho)
none Details | Review

Description Jamie Lavigne 2009-10-15 16:05:56 UTC
When opening text files with a .m extension (MATLAB/Octave), gedit uses syntax highlighting for Objective-C, when it should use the highlighting for Octave.  This was previously submitted as Ubuntu bug #451816: https://bugs.launchpad.net/ubuntu/+source/gtksourceview2/+bug/451816
Comment 1 Ignacio Casal Quinteiro (nacho) 2009-10-16 15:15:01 UTC
Does this happen for all *.m files? Could you provide an example of file for that?
If you make gvfs-info file.m does it provide the right content type?
Comment 2 Jamie Lavigne 2009-10-16 15:32:29 UTC
Created attachment 145617 [details]
A sample MATLAB script

Yes, it seems to happen for all .m files, even empty ones.  The following is the output of running gvfs-info on the attached file:

display name: mytranspose.m
edit name: mytranspose.m
name: mytranspose.m
type: regular
size: 74
attributes:
  standard::name: mytranspose.m
  standard::type: 1
  standard::size: 74
  standard::allocated-size: 4096
  standard::display-name: mytranspose.m
  standard::edit-name: mytranspose.m
  standard::copy-name: mytranspose.m
  standard::content-type: text/x-matlab
  standard::icon: text-x-matlab, gnome-mime-text-x-matlab, text-x-generic
  standard::fast-content-type: text/x-objcsrc
  unix::device: 2054
  unix::inode: 6897811
  unix::nlink: 1
  unix::uid: 1000
  unix::gid: 1000
  unix::rdev: 0
  unix::mode: 33188
  unix::block-size: 4096
  unix::blocks: 8
  time::modified: 1255706831
  time::modified-usec: 0
  time::access: 1255706930
  time::access-usec: 0
  time::changed: 1255706928
  time::changed-usec: 0
  etag::value: 1255706831:0
  id::file: l2054:6897811
  id::filesystem: l2054
  owner::user: njl
  owner::user-real: Jamie Lavigne
  owner::group: njl
  access::can-read: TRUE
  access::can-write: TRUE
  access::can-execute: FALSE
  access::can-rename: TRUE
  access::can-delete: TRUE
  access::can-trash: TRUE
Comment 3 Ignacio Casal Quinteiro (nacho) 2009-10-16 23:08:40 UTC
Created attachment 145637 [details] [review]
glob vs mime type

This should fix the bug. Still needs some review.
Comment 4 Paolo Borelli 2009-10-17 13:20:16 UTC
Review of attachment 145637 [details] [review]:

::: gtksourceview/gtksourcelanguagemanager.c
@@ +410,3 @@
 }
 
+static GList *

microoptimizaion, use GSList

@@ +413,1 @@
 pick_lang_for_filename (GtkSourceLanguageManager *lm,

rename to pick_langs_for_filename

@@ +528,3 @@
+	/* On Unix "content type" is mime type */
+	lang = pick_lang_for_mime_type (lm, content_type);
+#else

so basically on unix we are just calling pick_lang_for_mime_type... for consistency I'd say to rename this function to pick_lang_for_mime_type, and rename pick_lang_for_mime_type to pick_lang_for_mime_type_real

@@ -596,3 +612,3 @@
 	{
-#ifndef G_OS_WIN32
-		/* On Unix "content type" is mime type */
+		/* Many glob matches we have to pick the one that matches
+		   the content type*/

I'd like a more verbose commentary that explains the algo...

at the top of the function (where there was the old TODO comment), I'd write:

/* Glob take precedence over mime match. Mime match is used in the following cases
   - to pick among the list of glob matches
   - to refine a glob match (e.g. glob is xml and mime is an xml dialect)
   - no glob matches
*/

Then at this point

/* Use mime to pick among glob matches */

@@ -597,7 +613,7 @@
-#ifndef G_OS_WIN32
-		/* On Unix "content type" is mime type */
-		lang = pick_lang_for_mime_type (lm, content_type);
... 4 more ...
+		/* Many glob matches we have to pick the one that matches
+		   the content type*/
+		if (content_type != NULL)
... 4 more ...

There is a small logical bug here... it is not very important in practice, but it would be cleaner to have a consistent behavior. With you code if there are many glob matches and no content type, the first glob match is returned, but if there are many glob matches, content type != NULL, but the content type does not match any of the glob candidates, the *last* glob lang is returned.

@@ +633,3 @@
+					{
+						if (!g_content_type_equals (content_type, content))
+							lang = get_lang_for_mime_type (lm, content_type);

Can this fail? maybe we should check and in that case keep the current lang:

if (!g_content_type_equals (content_type, content))
{
   GtkSourceLanguage *mimelang;

   mimelang = get_lang_for_mime_type (lm, content_type);
   if (mimelang != NULL)
      lang = mimelang;
}
Comment 5 Ignacio Casal Quinteiro (nacho) 2009-10-17 23:19:15 UTC
Created attachment 145702 [details] [review]
glob vs mime type v2

I think it fixes almost everything.

about this comment:
There is a small logical bug here... it is not very important in practice, but
it would be cleaner to have a consistent behavior. With you code if there are
many glob matches and no content type, the first glob match is returned, but if
there are many glob matches, content type != NULL, but the content type does
not match any of the glob candidates, the *last* glob lang is returned.

I make after:
if (content_type != NULL)
+               {
/* Only one glob matched */
+               lang = GTK_SOURCE_LANGUAGE (langs->data);

So it is going to get the first of the list not the last.
Is it correct or it is because I create the list by making prepend and then I do not make the reverse? For this case the reverse wouldn't be neccessary as we are looking in the list.
Comment 6 Ignacio Casal Quinteiro (nacho) 2009-10-18 13:40:35 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.