Re: [zypp-devel] [yast-devel] patch disk usage (was: repodata.c - clarification)
Hi all any news about the patch? Ciao Roberto On Tue, Aug 26, 2008 at 9:16 AM, Stanislav Visnovsky <visnov@suse.cz> wrote:
Hi Roberto!
Thanks a lot for the patch! Unfortunatelly, our maintainer for the package selector is on vacation.
Maybe someone else from the team can review the patch?
Stano
Dňa Friday 22 August 2008 23:32:51 Roberto Mannai ste napísal:
Hi Stano
I believe zypp-devel is the proper mailing list for this.
Yes, thank you, they clared the question.
I append the patch, trying to resolve the enhancement in: https://bugzilla.novell.com/show_bug.cgi?id=399239 - "YaST's software installation modules should show disk space modification".
It adds the string " - Installing x MB" or " - Removing x MB" to the tooltip on the "Free space" column (in the disk usage widget) .
The disk usage variation will work for the same packages that change the disk usage bar; the zypp-devel mailing list pointed out that the repositories with disk usage attribute are those soddisfying the command: dumpsolv /var/cache/zypp/solv/MYREPO/solv | grep diskusage
Here the patch, feel free to change (or throw away :)):
Index: qt/src/QY2DiskUsageList.cc =================================================================== --- qt/src/QY2DiskUsageList.cc (revision 50308) +++ qt/src/QY2DiskUsageList.cc (working copy) @@ -229,9 +229,20 @@ if ( nameCol() >= 0 ) setText( nameCol(), name() ); if ( deviceNameCol() >= 0 ) setText( deviceNameCol(), deviceName() ); } +
- if ( usedSizeCol() < 0 ) - setToolTip( freeSizeCol(), _( "Used %1" ).arg( usedSize().form( 0, 1, true ).c_str() ) ); + if ( usedSizeCol() < 0 ) { + QString message = _( "Used %1" ).arg( usedSize().form( 0, 1, true ).c_str()); + + if(deltaSize() > 0){ + message = message + _( " - Installing %1" ).arg( deltaSize().form( 0, 1, true ).c_str() ); + + } else if(deltaSize() < 0){ + message = message + _( " - Removing %1" ).arg( deltaSize().form( 0, 1, true ).c_str() ); + + } + setToolTip( freeSizeCol(), message); + } }
Index: qt/src/QY2DiskUsageList.h =================================================================== --- qt/src/QY2DiskUsageList.h (revision 50308) +++ qt/src/QY2DiskUsageList.h (working copy) @@ -132,6 +132,13 @@ * is the default implementation. **/ virtual FSize freeSize() const; + + /** + * The delta partition space, after installation / removal. + * Derived classes may choose to reimplement this method. + * Default implementation returns 0. + **/ + virtual FSize deltaSize() const { return 0; }
/** * The currently used percentage ( 0..100 ) of this partition. Index: qt-pkg/src/YQPkgDiskUsageList.cc =================================================================== --- qt-pkg/src/YQPkgDiskUsageList.cc (revision 50323) +++ qt-pkg/src/YQPkgDiskUsageList.cc (working copy) @@ -114,9 +114,16 @@ const ZyppPartitionDu & partitionDu = *it; YQPkgDiskUsageListItem * item = _items[ QString::fromUtf8(partitionDu.dir.c_str()) ];
- if ( item ) - item->updateDuData( partitionDu ); - else + if ( item ) { + int usedBefore = item->usedSize(); + item->updateDuData( partitionDu ); + int usedAfter = item->usedSize(); + int usedDelta= usedAfter - usedBefore; + if (usedDelta != 0){ + item->setDeltaSize(usedDelta); + } + + } else yuiError() << "No entry for mount point " << partitionDu.dir << endl; }
@@ -258,6 +265,7 @@ , _partitionDu( partitionDu ) , _pkgDiskUsageList( parent ) { + _deltaSize = 0; yuiDebug() << "disk usage list entry for " << partitionDu.dir << endl; }
@@ -268,8 +276,20 @@ return FSize( _partitionDu.pkg_size, FSize::K ); }
+void +YQPkgDiskUsageListItem::setDeltaSize(FSize aDelta) +{ + _deltaSize += aDelta; +}
FSize +YQPkgDiskUsageListItem::deltaSize() const +{ + return _deltaSize; +} + + +FSize YQPkgDiskUsageListItem::totalSize() const { return FSize( _partitionDu.total_size, FSize::K ); Index: qt-pkg/src/YQPkgDiskUsageList.h =================================================================== --- qt-pkg/src/YQPkgDiskUsageList.h (revision 50323) +++ qt-pkg/src/YQPkgDiskUsageList.h (working copy) @@ -234,6 +234,19 @@ * Reimplemented from QY2DiskUsageListItem. **/ virtual FSize totalSize() const; + + /** + * The delta partition space, after installation / removal. + * + * Reimplemented from QY2DiskUsageListItem. + **/ + virtual FSize deltaSize() const; + + /** + * Set delta partition space, after marking packages for installation / removal. + * + **/ + virtual void setDeltaSize(FSize aSize) ;
/** * The name to display for this partition ( the mount point ). @@ -263,6 +276,7 @@
ZyppPartitionDu _partitionDu; YQPkgDiskUsageList * _pkgDiskUsageList; + FSize _deltaSize; };
Best regards Roberto Mannai
Roberto Mannai wrote:
Hi all any news about the patch?
Ciao Roberto
On Tue, Aug 26, 2008 at 9:16 AM, Stanislav Visnovsky <visnov@suse.cz> wrote:
Hi Roberto!
Thanks a lot for the patch! Unfortunatelly, our maintainer for the package selector is on vacation.
Maybe someone else from the team can review the patch? ... (see the previous mail/s)
Best regards Roberto Mannai
Stefan, What do you think of this patch? See http://lists.opensuse.org/yast-devel/2008-08/msg00059.html Thx && Bye Lukas -- Lukas Ocilka, YaST Developer (xn--luk-gla45d) ----------------------------------------------------------------- Ano, ano. Moudry rozkaz. Sam jsem nemel v tech gratulacich jasno.
Roberto Mannai wrote:
Hi all any news about the patch?
Ciao Roberto
I am sorry. It will be reviewed soon. There was other stuff to do, so the patch stayed in the queue. Duncan -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
Hola!
any news about the patch?
I had a look at the patch, compiled y2-qt packager with it, it works fine, also code-wise, there's nothing much to add to it. But: What I particularly don't like is the concept of solving the whole problem ( i.e. informing the user about how much disk space will be taken/freed if s/he accepts the changes made in this session). I mean, I have to un-hide the disk space graph widget, hover the mouse over current partition and only then I can see a tiny tooltip, isn't it a bit ... clumsy? Yes, it is probably the least obtrusive change to package selector UI ( no annoying pop-ups, no additional buttons in already too much crowded screen ... ), but it is so badly accesible that if I don't know exactly what I'm looking for, I have no chance to find it. Not to mention the fact that I'd very much liked to see some solution usable in all package manager UIs, so we don't have to recycle and reimplement the code over and over again (in all package manager UIs). I filed this enhancement request https://bugzilla.novell.com/show_bug.cgi?id=410897 so that the functions calculating disk usage changes are made available outside zypper and all package managers (UI or CLI) can use them i.e. can ask libzypp about current status when user/application requests it. B. -- \\\\\ Katarina Machalkova \\\\\\\__o YaST developer __\\\\\\\'/_ & hedgehog painter
Ciao Katarina On Thu, Sep 18, 2008 at 12:08 PM, Katarina Machalkova <kmachalkova@suse.cz> wrote:
What I particularly don't like is the concept of solving the whole problem ( i.e. informing the user about how much disk space will be taken/freed if s/he accepts the changes made in this session). I mean, I have to un-hide the disk space graph widget, hover the mouse over current partition and only then I can see a tiny tooltip, isn't it a bit ... clumsy? Yes, it is probably the least obtrusive change to package selector UI ( no annoying pop-ups, no additional buttons in already too much crowded screen ... ), but it is so badly accesible that if I don't know exactly what I'm looking for, I have no chance to find it.
Yes it may be clumsy, but look at what Stefan Hundhammer write here: https://bugzilla.novell.com/show_bug.cgi?id=399239 << Quite a number of people wanted this invisible by default; they say (I don't agree to that) that this makes this dialog more complex. So it was decided that we remove it.
Many users asked for that, and a bit of us (I and Stefan :), at least) don't agree. I thougtht that demanding / interested users, as well as open the table widget, could also trigger the tooltip :)
Not to mention the fact that I'd very much liked to see some solution usable in all package manager UIs, so we don't have to recycle and reimplement the code over and over again (in all package manager UIs). I filed this enhancement request https://bugzilla.novell.com/show_bug.cgi?id=410897 so that the functions calculating disk usage changes are made available outside zypper and all package managers (UI or CLI) can use them i.e. can ask libzypp about current status when user/application requests it.
Generally speaking I agree with code refactoring practice, I hope yast will be better ;) So are you going to discard the patch? Ciao Roberto -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
On Donnerstag, 18. September 2008, Roberto Mannai wrote:
So are you going to discard the patch?
No. It is really welcome (thanks for your work!), and I'd say we should really include it. It's not as if it would hurt anybody if it's done that way. That doesn't mean that there might not be other or even better ways. Some day we might want to do it some other way. But really, doing it like this is much better than not doing it at all. I just didn't get around yet to have a look at the patch; it's in the queue. You know, I am one of those old-fashioned people who try to do at least some basic testing before sending new stuff out to the world. ;-} CU -- Stefan Hundhammer <sh@suse.de> Penguin by conviction. YaST2 Development SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg) Nürnberg, Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
Hi,
So are you going to discard the patch?
I'm not the one who makes the final decision, not at all. I only wanted to provide some feedback, which is my personal point of view and might not necessarily correspond to that of y2-qt-pkg maintainer. Besides zero possibility to re-use the code that calculates 'delta size' somewhere else (outside qt-pkg), I objected that accesibility and visibility of this particular bit of information is, using tooltip approach, not optimal (I wonder how many users will actually discover it is there). If I had to choose between seeing nice coloured graph of all relevant partitions and tiny tooltips with disk usage summary, and disk size summary text ( 'X MB downloaded, Y MB installed, Z MB freed', HTML-formatted). I would certainly opt for the latter. It is more important for me to know what will happen now, if I download all the packages I've selected, than to see how full is my disk -> the graph could be displayed only on request (e.g. from the menu - curses do it that way) and when the disk space is running out. B. -- \\\\\ Katarina Machalkova \\\\\\\__o YaST developer __\\\\\\\'/_ & hedgehog painter
participants (5)
-
Duncan Mac-Vicar Prett -
Katarina Machalkova -
Lukas Ocilka -
Roberto Mannai -
Stefan Hundhammer