sfstoolbox / sfs-matlab

SFS Toolbox for Matlab/Octave
https://sfs-matlab.readthedocs.io
MIT License
96 stars 39 forks source link

Add optional message to progress_bar() #170

Closed hagenw closed 6 years ago

hagenw commented 6 years ago

This pull request is split into two commits. 2d79dcf simplifies the actual code of progress_bar() to allow an easier configuration of the actual progress bar. In addition, it adds an optional third argument message which is presented instead of the default Progress message. The first commit changes nothing on the output of all the progress_bar() calls we have so far in the toolbox.

In the second commit (fe3a46f) I tried another progress bar style. Employing a wider bar and using only #. This is just an proposal, I'm not really decided if I would favor the new version over the old one ;) @fietew: any opinion on this.

Here is an example of both styles:

>> for ii=1:30, progress_bar(ii,30); pause(0.1); end

old_bar new_bar

fietew commented 6 years ago

The optional message is a good idea. It is possible to automatically get the name of the function calling progress_bar? I think this would be a better default value for the message.

Regarding the style, I would prefer the shorter progress bar to not get into trouble with linebreaks in the MATLAB console.

hagenw commented 6 years ago

When I started with this, I was also hoping to get automatically the name of the calling function, but that is not too easy. One has to use dbstack() to get those information, which I thought is too much overhead. And it would also not help in all cases, because in some cases "Progress" would be simply replaced by "sound_field_mono" or "sound_field_imp" where most of the looping is happening. That was the reason why I simply added the optional message as an easy compromise.

I would then propose that I delete the second commit and you do the rebase merge afterwards?

fietew commented 6 years ago

I would then propose that I delete the second commit and you do the rebase merge afterwards?

okay

hagenw commented 6 years ago

It's ready.