[yast-devel] Progress::New() can be called recursively now
Hi all, I changed the internals of Progress:: module to support multiple (nested) invocations of Progress::New() function. The change has been submitted in yast2-2.16.22. Here are some details about my change. Introduction ------------ So what's the change is about? Imagine two yast modules which use Progress:: in their Write() function. Now you would like to use these two modules in your client and display a progress in your Write() function. The problem is that the there will be displayed two independent progresses (from 0% to 100%), one for each Write() call and that's not what user expects - progress 100% doesn't mean that the work is finished! So the usual solution is to disable Progress:: before calling another Write(), but the details about the nested progress are lost. The new functionality allows you to create a "parent progress" which has two stages. The progress values will be recalculated so that the first Write() call will display progress from 0% to 50% the second one from 50% to 100%. The level is "unlimited", the first Write() can call another nested two Write() functions which will be recalculated to 0%-25% and to 25%-50%, and so on... Why it was changed? ------------------- I wanted to fix bug #352007. The package callbacks now use Progress:: module and many yast clients already use Progress:: at start and starting the package manager is usually one step in their progress (e.g. online_update, sw_single). Now the progress bar displays correct (recalculated) values for the package manager steps. This allows us to build new modules on top of others without loosing details in Read() and Write() functions. If you do not want to display some details you can still hide them using Progress::set(false). Internals of Progress:: ----------------------- Progress:: has now an internal stack with progress state, Progress::New() checks if there is already a progress running, if there is no progress it behaves as it did in the past. When there is already a progress running then Progress::New() pushes the current state to the stack and all progress values are recalculated according to the previous states. (In this case the stages are not displayed, they are part of the parent stage.) Progress::Finish() pops the state from the stack so the previous internal state is set back. Examples -------- There are some examples in SVN, see yast2/library/wizard/doc/examples/progress_*.ycp files, I think this is the best way to learn how it works and how to use the new feature... -- Best Regards Ladislav Slezák Yast Developer ------------------------------------------------------------------------ SUSE LINUX, s.r.o. e-mail: lslezak@suse.cz Lihovarská 1060/12 tel: +420 284 028 960 190 00 Prague 9 fax: +420 284 028 951 Czech Republic http://www.suse.cz/ -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
Hello, On Jan 23 09:17 Ladislav Slezak wrote (shortened):
The new functionality allows you to create a "parent progress" which has two stages. The progress values will be recalculated so that the first Write() call will display progress from 0% to 50% the second one from 50% to 100%.
Can the "parent progress" only have two stages? What about three or more stages?
The level is "unlimited", the first Write() can call another nested two Write() functions which will be recalculated to 0%-25% and to 25%-50%, and so on...
I wonder whether this recalculation is really right. Assume the hierarchy is: parent -> child1, child2 -> child1, grandchild1, grandchild2 If I understand you correctly, the recalculated progress values are: child1: 0% to 50% grandchild1: 50% to 75% grandchild2: 75% to 100% I think that usually the user assumes that the progress value match to the time it takes to do the task. But because of your recalculation it looks as if child1 needs twice the time of a grandchild. I think a better recalculation would be to devide it equally regardless if it is a child or grandchild, i.e: child1: 0% to 33% grandchild1: 33% to 66% grandchild2: 66% to 100% Of course this doesn't make sure that the progress values match really well to the actual time because it assumes that all children need the same time but I think this is better than introduce "artificial" differences. Kind Regards Johannes Meixner -- SUSE LINUX Products GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany AG Nuernberg, HRB 16746, GF: Markus Rex -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
Johannes Meixner wrote:
Hello,
On Jan 23 09:17 Ladislav Slezak wrote (shortened):
The new functionality allows you to create a "parent progress" which has two stages. The progress values will be recalculated so that the first Write() call will display progress from 0% to 50% the second one from 50% to 100%.
Can the "parent progress" only have two stages? What about three or more stages?
No, problem, "any" number of stages is supported, each stage is dynamically partitioned at runtime if a nested progress is started.
The level is "unlimited", the first Write() can call another nested two Write() functions which will be recalculated to 0%-25% and to 25%-50%, and so on...
I wonder whether this recalculation is really right.
Assume the hierarchy is:
parent -> child1, child2 -> child1, grandchild1, grandchild2
If I understand you correctly, the recalculated progress values are:
child1: 0% to 50% grandchild1: 50% to 75% grandchild2: 75% to 100%
Yes, your assumption is right.
I think that usually the user assumes that the progress value match to the time it takes to do the task.
But because of your recalculation it looks as if child1 needs twice the time of a grandchild.
I think a better recalculation would be to devide it equally regardless if it is a child or grandchild, i.e:
child1: 0% to 33% grandchild1: 33% to 66% grandchild2: 66% to 100%
In this case you need to know how many children there will be _before_ the progress is started. And this is the problem. If you call Foo::Write() function you do not know how many nested progresses there will be and how many steps they will have (and even you know that it can be changed in the future). I think the current implementation is good enough, the most important thing for user is usually to see a change in the progress. The exact amount of the change is usually not very important. Ladislav -- Best Regards Ladislav Slezák Yast Developer ------------------------------------------------------------------------ SUSE LINUX, s.r.o. e-mail: lslezak@suse.cz Lihovarská 1060/12 tel: +420 284 028 960 190 00 Prague 9 fax: +420 284 028 951 Czech Republic http://www.suse.cz/ -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
Hello, On Jan 23 13:04 Ladislav Slezak wrote (shortened):
Johannes Meixner wrote: ...
I think a better recalculation would be to devide it equally regardless if it is a child or grandchild, i.e:
child1: 0% to 33% grandchild1: 33% to 66% grandchild2: 66% to 100%
In this case you need to know how many children there will be _before_ the progress is started. ... I think the current implementation is good enough
In this case the current implementation is perfect which means "as good as possible" because obviously nothing can be better than possible. Kind Regards Johannes Meixner -- SUSE LINUX Products GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany AG Nuernberg, HRB 16746, GF: Markus Rex -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org For additional commands, e-mail: yast-devel+help@opensuse.org
Hola!
I changed the internals of Progress:: module to support multiple (nested) invocations of Progress::New() function. The change has been submitted in yast2-2.16.22. Here are some details about my change.
With Lada Slezak's new implementation of nested progresses on the stack, entirely new kind of bugs surfaced ;-) so I'd like all YCP module maintainers to read the following and check if your module is not affected by this issue *) So far, yast2-network and yast2-users are ... will fix y2-network soonish. Every series of Progress::NextStage() calls in your Read() function should be properly terminated with Progress::Finish() after the last stage. The same basically holds true for Write() function, but there it does not hurt so much. If you fail to do so, Progress module does not clean up the stack by itself (because it does not know that you actually want to finish). Thus, when your Write() function wants to display a new progress, it thinks there are some progresses on the stack, wants to be nested into existing progress and tries to update (non-existing) progress bar. This throws a libyui exception and causes evil red popup to appear. I was thinking about adding some check whether progress bar WidgetExists into the function checking whether we already have some progress running, but that's not 100% correct solution. It just prevents libyui from throwing ex. after an attempt to update non-existing widget. Someone/something must reset progress counter. An ideal candidate is Progress::Finish() - but in that case the module maintainers must take care of proper nesting of the progresses, not Progress.ycp module itself. Any ideas? (*) unfortunately, we'll probably not manage to fix for tomorrow's alpha2 -- \\\\\ Katarina Machalkova \\\\\\\__o YaST developer __\\\\\\\'/_ & hedgehog painter
Katarina Machalkova napsal(a):
Hola! ... Every series of Progress::NextStage() calls in your Read() function should be properly terminated with Progress::Finish() after the last stage. The same basically holds true for Write() function, but there it does not hurt so much.
If you fail to do so, Progress module does not clean up the stack by itself (because it does not know that you actually want to finish). Thus, when your Write() function wants to display a new progress, it thinks there are some progresses on the stack, wants to be nested into existing progress and tries to update (non-existing) progress bar. This throws a libyui exception and causes evil red popup to appear.
I was thinking about adding some check whether progress bar WidgetExists into the function checking whether we already have some progress running, but that's not 100% correct solution. It just prevents libyui from throwing ex. after an attempt to update non-existing widget. Someone/something must reset progress counter. An ideal candidate is Progress::Finish() - but in that case the module maintainers must take care of proper nesting of the progresses, not Progress.ycp module itself. Any ideas?
From installation point of view, not solving this issue in general might break the installation workflow in case some plug-in module is broken. There are many Read() and Write() processes called but installation can't control them, it just calls them and evaluate their return value. Every single plug-in can break it. In my opinion, if (UI::WidgetExists (...)) would be sufficient (with error in y2log if it goes wrong). L.
participants (4)
-
Johannes Meixner -
Katarina Machalkova -
Ladislav Slezak -
Lukas Ocilka