PatchBay update for Haiku

[Pete]

I've generated a new version of PatchBay, edited to conform (hopefully!) to Haiku coding standards, and using Haiku's ToolTip mechanism rather than the custom code in the original.

I've added a vector icon for the "Unknown Devices" it displays, and given it a Desktop icon as well.

This ticket is a follow-on from #9159, but following Philippe's suggestion there I've started a new ticket.

As noted there, this code should be in .../apps rather than .../tests/... so if it's OK, can someone please move it there in the repository?

[Pete]

Updates to the original PatchBay app

[waddlesplash]

Two questions:

• Will PatchBay appear in Preferences menu after this patch is applied and source moved to .../apps?
• Can you compile it on GCC4 after this? If not, could it be fixed at the same time?

Thank you!

[phoudoin]

Thanks Pete for the (git format-)patch. After a 2 years break, I'm setting up a new Haiku system (just bought a new Asus Vivobook S300CA) and hope to be able to work again on Haiku stuffs this week. My coding style guide skills are a bit rusty, too. So, stay tuned.

One remark/question : should we keep the x-vnd.Be.PatchBay signature (for compatibility sake, some existing midi apps may launch it?) or endorse it has our now, x-vnd.Haiku.PatchBay ?

Seems that there is now an app_name_catalog_entry resource to set up too for application name localisation:

resource app_name_catalog_entry "x-vnd.Haiku-PatchBay:System name:PatchBay";

[bonefish]

Replying to Pete:

As our patch guidelines say and people have written on the list, please use git format-patch to create the patch. This will cause your contribution to be credited correctly and since the patch will already include the commit message, it is also less work for the committer.

As noted there, this code should be in .../apps rather than .../tests/... so if it's OK, can someone please move it there in the repository?

Please just do it yourself (in another commit). Unlike subversion, git has no trouble conveying such a change via a patch.

[Pete]

git format-patch version of updates

[Pete]

I-O-M files of the PatchBay icons

[Pete]

OK, I've made a git format-patch version of the updates which seems to be correct (moved from src/tests/kits/midi to src/apps). I've also attached a zip of the I-O-M files I created for the app's icons, as they're otherwise only on my machine.

[korli]

Pete, do you intend to fix the code style issues in another patch? We have a style checker script in src/tools/checkstyle to help for the task.

[Pete]

Replying to korli:

Pete, do you intend to fix the code style issues in another patch? We have a style checker script in src/tools/checkstyle to help for the task.

Eh...? I thought that was what I had done... (:-/) (The first thing I did was to run the checker! ... before doing the other rewriting -- which I thought adhered to the style codes,)

[korli]

OK, a few things:

• #ifndef _EndpointInfo_h, _EndpointInfo_h is usually uppercased, dunno if it's in the coding style guidelines..
• In header files, there is no blank line between license text and header guard.
• there are a few occurrences of braces on their own lines along with a for or a if.
• the copyright license header doesn't follow the guidelines.
• "if (box)" should be "if (box != NULL)"
• "if (producer) producer->Release();" should be on two lines
• separate #include group with a newline

Improvements to the checkstyle script are welcome too :)

[Pete]

Replying to korli:

OK, a few things:

• #ifndef _EndpointInfo_h, _EndpointInfo_h is usually uppercased, dunno if it's in the coding style guidelines..

Oops, yes -- shouldn't have missed that...

• In header files, there is no blank line between license text and header guard.

OK

• there are a few occurrences of braces on their own lines along with a for or a if.

OK

• the copyright license header doesn't follow the guidelines.

Would you like to expand on that? I tried to figure out what was appropriate, and that was my best guess.

• "if (box)" should be "if (box != NULL)"

Missed that too. [Maybe because that's one of the guidelines I don't agree with! (:-/)]

• "if (producer) producer->Release();" should be on two lines

OK

• separate #include group with a newline

OK

Improvements to the checkstyle script are welcome too :)

Yep... None of the above are caught by checkstyle.py. I'm probably not the one to make any improvements, though. (:-))

It really is hard for someone who normally uses a different style to notice all the variances in a large swathe of code, but I'll do my best.

[Pete]

2nd attempt to bring code up to style guide

[Pete]

Sorry -- I did these code updates a month ago, but got diverted and never submitted them here. New patch (additional to the previous, not a replacement) attached.

I've tried to address all the style-guide concerns I could find, previously mentioned and unmentioned, except for the Copyright header -- where I have no idea what should be fixed.

Be glad if it can get committed now.

[Pete]

Can someone please commit this, so it can be made available as an optional package? It's been three months now... Thanks.

[Pete]

Philippe, thanks for taking this on.

I was going to look into whether it would be better as a bundled app or an optional package. I remember you prefer the former (https://dev.haiku-os.org/ticket/9159#comment:11), and I think I agree -- it's only ~80kB.

I'd provide a patch to add it to the generated set, but I can't quite figure out where that is controlled. It's in the .../src/apps/jamfile, but that doesn't seem to cause it to be compiled or added to the image (similarly with a few others, like 3DMov). Where is this set?

[phoudoin]

In build/jam/HaikuImage, SYSTEM_APPS variable?

[phoudoin]

Patches applied and PatchBay promoted to Haiku default image's system demos in hrev45956.