HaikuDepot should have an advanced tab while opening package files directly

[kallisti5]

HaikuDepot now is the default program for package files. An advanced tab should be added to display the advanced package information and contents when package files are opened with HaikuDepo directly.

For example, open a package with expander and choose "Show contents" This information is infinitely useful while troubleshooting ports.

[diver]

I second that, had to close HaikuDepot and open Expander to see what's inside too many times.

[TigerKid001]

I'd like to work on this enhancement. Just one question, should it be a checkbox option below the package-info or a tab in the package-info view.

[humdinger]

I'd prefer another tab, maybe "Package details".

AFAIK those package details are only available when a package is locally stored, i.e. packages available online at the Haiku Depot Server don't provide that information. In that case the tab title might be ghosted and the tab view display a note like "Package details are only available after the package is downloaded/installed.".

Since the DESCRIPTION can be quite long and is already available from the "About" tab, it can be omitte from the details tab.

[stippi]

Replying to TigerKid001:

I'd like to work on this enhancement. Just one question, should it be a checkbox option below the package-info or a tab in the package-info view.

Thanks for your interest in working on this! I had plans to do it myself, but I'll gladly step back and offer myself as a mentor, if you need me to.

I would propose a tab "Contents" which shows for installed packages or locale package files. There is already code in place which iterates package contents via the Package Kit APIs to find a Deskbar link in a given package. See ​http://cgit.haiku-os.org/haiku/tree/src/apps/haikudepot/model/PackageManager.cpp#n351 and how it's called from here ​http://cgit.haiku-os.org/haiku/tree/src/apps/haikudepot/model/PackageManager.cpp#n511

I would use similar code to populate a BOutlineListView on the contents page which matches the entries in the package with list items structured in a tree. These can even have icons (from entry icon attributes or entry mime types) and columns for entry size, modified date and so on. This should not be complicated to write (by which I mean you should approach this with confidence :-). If you get stuck, please don't hesitate to ask! And sorry for any delay in replying or reviewing patches upfront!

[stippi]

Hi TigerKid001: Have you made any progress?

[TigerKid001]

Hi! Sorry for the delay. I have made some progress. I have used the PackageListView for reference and written a class to populate the OutlineListView. Will be uploading a patch soon :-)

[TigerKid001]

Hi stippi: I think i have hit a bump, and need some guidance. My first doubt is whether the contents of installed packages are also to be listed, because I'm having trouble retrieving their install location path. Secondly, though the list is being populated with the contents, its not in the way expected. (A screenshot of the problem: ​http://imgur.com/4mGJAzP). Thanks for the help in advance :-)

[stippi]

Hi TigerKid001: I saw your code, thanks for working on this. You seem to be having some misconceptions about how pointers work and/or how some of the BColumnListView API works. When you ask the list, whether it already contains an item, the list searches the exact pointer. When you create a new BListItem instance with "new", the list cannot have a pointer to that newly created item. To find the parent item, you need to find the item manually from the list. But this seems expensive. It would be much more efficient, if you remember the parent list item for as long as you encounter entries with the same parent entry. In any case, what you see in the list view is a direct result of never finding any parent item (because you are doing it wrongly) and all entries are added to the top level of the list. If you need more help than that, I can probably find the time to rewrite that function. But for that, I would like a patch with the complete code, so I can test my changes more easily.

[TigerKid001]

Contents Tab patch

[TigerKid001]

Hi stippi: Thanks for pointing out the mistake. I've re-written the function and it now works good (​http://imgur.com/Cim6Gec). Uploaded a patch :-). Just one question though, should I try and add the icon and size info to the list.

[humdinger]

Nice, TigerKid001! Is it possible to have another "expandable" entry "Package info" at the top that shows all the other package info just without Description and Summary, i.e. for example all the Provides, Requires, Global writable files, Version, Copright etc.?

Is all this information available before downloading and installing the package?

[stippi]

Hi TigerKid0001,

Replying to TigerKid001:

Uploaded a patch :-).

Good work so far, I will hopefully find some time to review soon.

Just one question though, should I try and add the icon and size info to the list.

Well. As is, the list is a bit dull. I can imagine many features. As I mentioned earlier, there could be columns for additional information, like entry size or the link targets. Icons would be nice, either from the entry in the package if provided, or from the respective MIME types. There could be a tool bar above the list with expand all and collapse all icons.

It all depends on your motivation to work on these features. If you don't have time or can't work on them for other reasons, I will probably apply your patch soon and work on these features myself. I don't want to step on your toes, if you want to work on it, I'll gladly keep mentoring only.

[TigerKid001]

Replying to humdinger:

Is it possible to have another "expandable" entry "Package info" at the top that shows all the other package info just without Description and Summary, i.e. for example all the Provides, Requires, Global writable files, Version, Copright etc.?

Hi humdinger: the version and publisher info can be easily added, i'll check for the others.

Is all this information available before downloading and installing the package?

Currently, the patch only works for local packages. But I'm now trying to make it work for remote packages as well. I think the information it is available, so will add that too. stippi: Thank you for mentoring me on this one. I'll gladly continue working on this and upload patches with improved features :-)

[stippi]

TigerKid001: I've applied your patch in hrev48643, with some changes and fixes. I hope it doesn't cause you too much merging trouble in case you already continued to work on it.

I wonder why you didn't follow my advice to copy PackageListView.cpp as the basis for the PackageContentListView. BOutlineListView does not support columns, but it would be nice to have a more Tracker-like view with columns that show the package entry attributes (icon, size, permissions and link targets at the least).

[stippi]

Just a heads up: My solution to finding previously added entries by the BPackageEntry pointer is wrong. Apparently, BPackageEntry instances are re-used during visiting entries, so these pointers become stale. In the outline, this will result in entries being added to the first list item at the same level as the actual parent. I will try to think of something different, but it is getting late...

[TigerKid001]

Hi stippi: thanks for the heads up. The merging won't be a trouble either. And I remember you suggested PackageListView.cpp as a basis, but I was a little excited to just get it working first ;-). I'll now make it into a column list.

[stippi]

I've fixed the issue with finding the correct parent list item in hrev48658. I think I will leave it at that for the time being and wait for your next patch. Looking forward to it! :-)

[TigerKid001]

Hi stippi: Sorry for the long delay (winter-break over :| ). Anyways, I've been working on this and am uploading the patch for what I've done so far. It compiles alright and the column list is also created, but somehow, it is not being populated. On investigating, I have found that the control reaches the _ContentPopulatorThread method, but doesn't enter the while loop (may acquire_sem is not returning B_OK ?). I'm stuck here and can't find a way to fix it. Hope you find time to review this. :)

[stippi]

Thanks! Will have a look hopefully tonight. If you don't hear from me in reasonable time, please don't hesitate to bug me again!

[stippi]

The problem should be this: You've removed the call to _InitContentPopulator(); from the PackageContentsView constructor. And in SetPackage(), instead of this code block:

    if (fPackage == package) 
        return;
//  printf("PackageContentsView::SetPackage(%s)\n", 
//      package.Get() != NULL ? package->Title().String() : "NULL"); 

    Clear(); 
    { 
        BAutolock lock(&fPackageLock); 
        fPackage = package; 
    } 
    release_sem_etc(fContentPopulatorSem, 1, 0);

... you are now doing this:

    fPackage = package;  
    printf("\nPackage set to %s\n", (fPackage->Title()).String()); 

    printf("Initializing population\n"); 
    _InitContentPopulator(); 

This is completely different. You need to restore the original code. The call to release_sem_etc(fContentPopulatorSem, 1, 0); is what makes the content thread start to run. And the way the locking happens is also important. The way you changed the code, a new content populator thread is created for each package that is supposed to show. The original code created only one thread. And the thread is then never put to work. The original design creates only one thread and starts and stops it on demand.

[stippi]

Also, once you get the populator to run, you should notice that fPackagePath is not maintained correctly (it grows indefinitely for each entry?). I don't understand what you want to do with it.

[TigerKid001]

Thanks stippi, I fixed the populator and it now works fine. The contents are populated alright. But I've run into a new problem. The way I was trying to get the icon is incorrect (I always doubted it). Can you please suggest a way to get the icons? Currently, I'm making nodeinfo object from path of the entry, but the entries can't be accessed since they are inside the hpkg. Also, the permissions are being displayed in the "-rw-r--r--" fashion. Is that alright?

[stippi]

I would assume you need to extract the icon-attribute of the package entry to memory. I was going to have a look, but then I realized you have not updated the patch with your changes from comment:21.

[stippi]

While parsing the package contents, there is a hook method HandleEntryAttribute(BPackageEntry* entry, BPackageEntryAttribute* attribute). And BPackageEntryAttribute has a method Data(). I have not checked it, but I assume that is how you can get to the icon. You should be able to construct a SharedBitmap from the buffer and it would automatically parse the icon data, even non-vector icons.

[stippi]

... and for entries without an icon attribute, you would fall back to their MIME-type attribute and use the SharedBitmap constructor which resolves the system-icon for a specific installed MIME-type. There is probably an edge-case left, which is when the package itself would install a MIME-type into the system and contains files with that MIME-type. Then you would have to read the icon from the MIME-type entry from the package. But that could be left for bonus points later.

[TigerKid001]

Patch for contents tab using columnlist, but without icons.

[TigerKid001]

Hi stippi. I tried to add the icons to the list items as you suggested. I assumed that the hook functions(HandleEntry, HandleEntryAttribute, HandleEntryDone) are executed consecutively. So, in HandleEntryAttribute, I'm trying extract the icon by:

	int32 attr = (int32) attribute->Data().InlineData();
	SharedBitmap shared(attr);
	fPackageIcon = (BBitmap*) shared.Bitmap(SharedBitmap::SIZE_16);

and then make a PackageRow with this icon in HandleEntryDone function. But still, the icons are not showing up. Moreover, the list is not being filled correctly, but it was when when the list-populating code was in HandleEntry. Where am i going wrong?

[TigerKid001]

Patch for comment 25

[diver]

Btw, this patch doesn't apply to current revision.

[stippi]

Hi TigerKid001!

You cannot cast stuff around like that. What InlineData() gives you is a pointer to memory (I assume). What the SharedBitmap constructor (which you are using) expects is an int32 resource ID. It will take this ID and try to extract a resource which is bundled with the HaikuDepot executable. So this cannot work. But there is another SharedBitmap constructor which takes a BPositionIO. You need to construct a BMemoryIO or similar initialized from the InlineData() (the pointer to the memory buffer and the size of the memory block).

Also, fPackageIcon needs to be a reference to a SharedBitmap instead of a pointer to a BBitmap, so the memory ownership is clear. If you construct a SharedBitmap on the stack like that and take the pointer to the BBitmap which it owns (and which you are not allowed to mess with, just use via read-only access (which is indicated by the fact that the Bitmap() method returns const BBitmap*)), what happens when the shared variable goes out of scope (if you don't know what that means, just google it), is that its memory is free'd and the fPackageIcon pointer now points to free'd memory that can be overwritten at any time by other parts of the program.

I need to look more deeply into your patch to see why it has stopped working. Thanks for your work so far!

[TigerKid001]

Hi stippi! Thanks for the pointers. I didn't know InlineData() returned a pointer to memory, and the casts didn't feel right anyways. But I was unable to find any examples to use these functions, so just gave it a shot. I'll try again :)

[TigerKid001]

Hi stippi. Bothering you again. So, i changed the HandleEntryAttribute to

        BPackageData data = attribute->Data();
	BMemoryIO source(data.InlineData(),data.Size());
	const SharedBitmap &shared(source);
	fPackageIcon = shared;

Here, fPackageIcon is a reference to SharedBitmap. But since references must be bound to target objects when created, i dont know what to initialize it with. So, i'm stuck at that. Also, i moved the list-population part to a new function (which is called from HandleEntryAttribute) and the list is filled correctly now.

[stippi]

Sorry for the late reply... I realize that patch is not the current one. It would be good to always leave the latest patch attached to the ticket. That way, if I suddenly find myself with some free time, I am looking at the current state of the work.

Anyway, if fPackageIcon is a reference to a SharedBitmap, then you assign it like this:

fPackageIcon.SetTo(new(std::nothrow) SharedBitmap(source), true);

1) You have to create the SharedBitmap on the heap, since it is supposed to stay allocated when the function returns or the variable goes out of scope for other reasons.

2) You pass true to SetTo(), since creating the SharedBitmap with new creates an initial reference. You don't want to add another reference, so you pass true for the alreadyHasReference parameter.

[nielx]

Remove milestone for tickets with status = closed and resolution != fixed