Comment on attachment 128150 [details] [review]
patch that adds text/x-objc++src

Review of attachment 128150 [details] [review]:
-----------------------------------------------------------------

This needs to be a git-formatted patch, with a test case. See the HACKING file for details.

There is some kind of misunderstanding here.

This was supposed to be a wish-list/request ticket. I'll provide an ObjC++ file, but I do not currently have the time to go through a nitpicky review process for something that ought to be a simple copy/past with minor changes job that you can do in-house. The patch was only intended as a reference and a courtesy; if it puts people on the wrong foot I can remove it.

ObjC++ relates to C++ exactly in the way ObjC relates to C.

Created attachment 128157 [details]
a sample ObjC++ file, from the Qt 5.6.2 Cocoa platform plugin

(In reply to René J.V. Bertin from comment #2)
> There is some kind of misunderstanding here.
> 
> This was supposed to be a wish-list/request ticket. I'll provide an ObjC++
> file, but I do not currently have the time to go through a nitpicky review
> process for something that ought to be a simple copy/past with minor changes
> job that you can do in-house. The patch was only intended as a reference and
> a courtesy; if it puts people on the wrong foot I can remove it.

The courtesy is me maintaining this software, I don't have time to spend 20 minutes on each person's patches, because 20 minutes times ...

Please provide the git formatted patch, or I'll close this bug.

Created attachment 128161 [details]
patch

As requested.

I'll have learned my lesson.

Comment on attachment 128161 [details]
patch

That's nice of you to provide a git-formatted patch, but you didn't read the HACKING file:
- there's no need to add translations, those are merged from the .po files
- there's no test file, and tests/list isn't modified to include that file

Also make sure that "make check" passes.

Thanks.

Almost 2 years later there's still no ObjC++ entry, so please consider adding one, plus an x-objchdr type for completeness.

I've removed my patch as far as that's possible because it was only ever intended as an example of something that seems to work when I add it (manually) to the packages directory.

For the record:

I have tried to add the request test-case (to the current git/master/head), by

1) copying `simple-obj-c.m` to `simple-objc++.mm`
2) adding a line to `tests/list`:
```
simple-objc++.mm text/x-objc++src
```
3) adding the following to freedesktop.org.xml.in, after the entry for `x-objcsrc`
```
  <mime-type type="text/x-objc++src">
    <_comment>Objective-C++ source code</_comment>
    <sub-class-of type="text/x-c++src"/>
    <sub-class-of type="text/x-objcsrc"/>
    <magic priority="30">
      <match type="string" value="#import" offset="0"/>
    </magic>
    <glob pattern="*.mm"/>
  </mime-type>
```
4) installing xdgmime in the same parent.dir as the s-m-i source tree


Running `make check` gives the error that `simple-obj-c.m` is misinterpreted as `x-objc++src`. When I put the block from step 3) *before* the entry for `x-objcsrc` makes no difference. I get the opposite error when I also remove the magic code from the `x-objc++src` entry:
```
simple-objc++.mm, 'data' test: expected text/x-objc++src, got text/x-objcsrc
```

Suspicious I tried adding an equivalent test case for C++ by creating a file `simple-c++.cpp` containing a minimal valid C++ program. That gives exactly the same result:

```
simple-c++.cpp, 'data' test: expected text/x-c++src, got text/x-csrc
```

-----------------------
Two observations/conclusions:

- the existing test for ObjC is flawed and only succeeds because there is no confusion possible. The alternative conclusion might be that there is something seriously wrong with extension detection.
- C++ has no unittest, so why would I have to add one for ObjC++?

(In reply to René J.V. Bertin from comment #7)
> Almost 2 years later there's still no ObjC++ entry, so please consider
> adding one, plus an x-objchdr type for completeness.

See comment 6, I've been waiting for an updated patch.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xdg/shared-mime-info/issues/52.