On Tue, 14 Apr 2015, Luke Mewburn wrote: > On Mon, Apr 13, 2015 at 10:11:09AM -0400, Nicolas Pitre wrote: > | What if you suspend the task and push it into the background? Would be > | nice to inhibit progress display in that case, and resume it if the task > | returns to the foreground. > > That's what happens; the suppression only occurs if the process is > currently background. If I start a long-running operation (such as "git > fsck"), the progress is displayed. I then suspend & background, and the > progress is suppressed. If I resume the process in the foreground, the > progress starts to display again at the appropriate point.
I agree. I was just comenting on your suggestion about caching the in_progress_fd() result which would prevent that. > In the proposed patch, the stop_progress display for a given progress > (i.e. the one that ends in ", done.") is displayed even if in the > background so that there's some indication of progress. E.g. > Checking object directories: 100% (256/256), done. > Checking objects: 100% (184664/184664), done. > Checking connectivity: 184667, done. > This is the test 'if (is_foreground || done)'. Yes. And I think this is nice. > | Also the display() function may be called quite a lot without > | necessarily resulting in a display output. Therefore I'd suggest adding > | in_progress_fd() to the if condition right before the printf() instead. > > That's an easy enough change to make (although I speculate that the > testing of the foreground status is not that big a performance issue, > especially compared the the existing performance "overhead" of printing > the progress to stderr then forcing a flush :) Sure. But what I'm saying is that progress() may be called a thousand times and only one or two of those calls will result in an actual print-out. So it is best to test the foreground status only at that point. > Should I submit a revised patch with > (1) call in_progress_fd() just before the fprintf() as requested, and > (2) suppress all display output including the "done" call. > ? I'd suggest (1) but not (2). Nicolas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
