Bug#901758: diffoscope: leaves progress bar on terminal after completion
Hi Paul, > Attached a couple more patches. Also applied. :) Regards, -- ,''`. : :' : Chris Lamb `. `'` la...@debian.org / chris-lamb.co.uk `- ___ Reproducible-builds mailing list Reproducible-builds@alioth-lists.debian.net https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/reproducible-builds
Bug#901758: diffoscope: leaves progress bar on terminal after completion
On Sat, 2018-07-07 at 11:26 +0100, Chris Lamb wrote:
> Thanks, applied.
Attached a couple more patches.
--
bye,
pabs
https://wiki.debian.org/PaulWise
From 2c95bd3ec8e17c54fc2f8a1c4a1735d17df7ab55 Mon Sep 17 00:00:00 2001
From: Paul Wise
Date: Sat, 7 Jul 2018 16:31:24 +0800
Subject: [PATCH 1/2] Use the ProgressBar's idea of the terminal width instead
More likely to overwrite the correct number of characters.
Fixes: commit dfb0c190173dff953fe1481797486d0b3e09306e
---
diffoscope/progress.py | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/diffoscope/progress.py b/diffoscope/progress.py
index 286f9d9..5edfe84 100644
--- a/diffoscope/progress.py
+++ b/diffoscope/progress.py
@@ -232,10 +232,8 @@ class ProgressBar(object):
if self.erase_to_eol:
self.fd.buffer.write(self.erase_to_eol)
elif self.fd.isatty():
-from shutil import get_terminal_size
-width = get_terminal_size().columns
print(end='\r', file=self.fd)
-print(' ' * width, end='', file=self.fd)
+print(' ' * self.term_width, end='', file=self.fd)
else:
# Do not flush if nothing was written
return
--
2.18.0
From ed9cfc95bce9fc08aa5d0ac0d2ad1b709e9cb108 Mon Sep 17 00:00:00 2001
From: Paul Wise
Date: Sat, 7 Jul 2018 16:13:35 +0800
Subject: [PATCH 2/2] Do not delete the current terminal line for every
progress bar update
The progress bar already overwrites the entire line
so erasing it before that serves no purpose.
The erasure was also causing the progress bar to flicker
and wasn't portable to different types of terminals.
---
diffoscope/progress.py | 3 ---
1 file changed, 3 deletions(-)
diff --git a/diffoscope/progress.py b/diffoscope/progress.py
index 5edfe84..c0e140c 100644
--- a/diffoscope/progress.py
+++ b/diffoscope/progress.py
@@ -34,9 +34,6 @@ class ProgressLoggingHandler(logging.StreamHandler):
def emit(self, record):
try:
-# Delete the current line (i.e. the progress bar)
-self.stream.write("\r\033[K")
-self.flush()
super().emit(record)
if not self.progressbar.bar.finished:
self.progressbar.bar.update()
--
2.18.0
signature.asc
Description: This is a digitally signed message part
___
Reproducible-builds mailing list
Reproducible-builds@alioth-lists.debian.net
https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/reproducible-builds
Bug#901758: diffoscope: leaves progress bar on terminal after completion
On Sat, 2018-07-07 at 07:45 +0100, Chris Lamb wrote:
> I wasn't aware that was a possibility.
Yeah, different terminals have different capabilities.
Most modern ones of course can erase to EOL though.
> If you mean via --text=filename then the progress bar does not
> appear there.
I was thinking diffoscope &> output shell redirection.
> In that case, please could you provide a merge request?
I'm not interested in touching gitlab, patches attached.
--
bye,
pabs
https://wiki.debian.org/PaulWise
From 9fa755c81033148cc1a5a49bc4d2fe865ec6e0c8 Mon Sep 17 00:00:00 2001
From: Paul Wise
Date: Sat, 7 Jul 2018 16:13:35 +0800
Subject: [PATCH 1/3] Do not delete the current terminal line for every
progress bar update
The progress bar already overwrites the entire line
so erasing it before that serves no purpose.
The erasure was also causing the progress bar to flicker.
---
diffoscope/progress.py | 3 ---
1 file changed, 3 deletions(-)
diff --git a/diffoscope/progress.py b/diffoscope/progress.py
index e60188c..a584ef7 100644
--- a/diffoscope/progress.py
+++ b/diffoscope/progress.py
@@ -32,9 +32,6 @@ class ProgressLoggingHandler(logging.StreamHandler):
def emit(self, record):
try:
-# Delete the current line (i.e. the progress bar)
-self.stream.write("\r\033[K")
-self.flush()
super().emit(record)
if not self.progressbar.bar.finished:
self.progressbar.bar.update()
--
2.18.0
From f386b5e3dc5a5ebbc0dc06720b56f8af33ea7c20 Mon Sep 17 00:00:00 2001
From: Paul Wise
Date: Sat, 7 Jul 2018 16:31:24 +0800
Subject: [PATCH 2/3] Clear the progress bar after completion. (Closes:
#901758)
Handle terminals that do not support erasing the line by
filling the terminal with spaces.
Ignore output devices that are not terminals.
Implements: https://bugs.debian.org/901758
Requires: Python 3.2 for shutil.get_terminal_size
---
diffoscope/progress.py | 30 ++
1 file changed, 30 insertions(+)
diff --git a/diffoscope/progress.py b/diffoscope/progress.py
index a584ef7..215bb23 100644
--- a/diffoscope/progress.py
+++ b/diffoscope/progress.py
@@ -3,6 +3,7 @@
# diffoscope: in-depth comparison of files, archives, and directories
#
# Copyright © 2016 Chris Lamb
+# Copyright © 2018 Paul Wise
#
# diffoscope is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -20,6 +21,7 @@
import os
import sys
import json
+import signal
import logging
logger = logging.getLogger(__name__)
@@ -212,10 +214,38 @@ class ProgressBar(object):
# Remove after https://github.com/niltonvolpato/python-progressbar/pull/57 is fixed.
kwargs.setdefault('fd', sys.stderr)
super().__init__(*args, **kwargs)
+# Terminal handling after parent init since that sets self.fd
+if self.fd.isatty():
+from curses import tigetstr, setupterm
+setupterm()
+self.erase_to_eol = tigetstr('el')
+else:
+self.erase_to_eol = None
def _need_update(self):
return True
+def erase_line(self):
+if self.erase_to_eol:
+self.fd.buffer.write(self.erase_to_eol)
+elif self.fd.isatty():
+from shutil import get_terminal_size
+width = get_terminal_size().columns
+print(end='\r', file=self.fd)
+print(' ' * width, end='', file=self.fd)
+else:
+# Do not flush if nothing was written
+return
+self.fd.flush()
+
+def finish(self):
+self.finished = True
+self.update(self.maxval)
+# Clear the progress bar after completion
+self.erase_line()
+if self.signal_set:
+signal.signal(signal.SIGWINCH, signal.SIG_DFL)
+
self.bar = OurProgressBar(widgets=(
' ',
progressbar.Bar(),
--
2.18.0
From ae7c7acc52bc45a452555dd16932fc21419426f8 Mon Sep 17 00:00:00 2001
From: Paul Wise
Date: Sat, 7 Jul 2018 17:22:19 +0800
Subject: [PATCH 3/3] Erase the progress bar line when diffoscope is
interrupted
Otherwise cruft is left on the terminal after exit.
---
diffoscope/main.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/diffoscope/main.py b/diffoscope/main.py
index 375cdc2..e2fb945 100644
--- a/diffoscope/main.py
+++ b/diffoscope/main.py
@@ -459,6 +459,8 @@ def main(args=None):
post_parse(parsed_args)
sys.exit(run_diffoscope(parsed_args))
except KeyboardInterrupt:
+if log_handler:
+log_handler.progressbar.bar.erase_line()
logger.info('Keyboard Interrupt')
sys.exit(2)
Bug#901758: diffoscope: leaves progress bar on terminal after completion
tags 901758 - pending
thanks
> > +self.fd.write("\033[K")
>
> This isn't going to work if the terminal being used doesn't support
> that escape sequence
I wasn't aware that was a possibility.
> or if the user requested a progress bar but directed output to
> a file
If you mean via --text=filename then the progress bar does not
appear there.
> I'd suggest the approach I mentioned in the initial bug report
> instead
In that case, please could you provide a merge request? Reverted my
change in:
https://salsa.debian.org/reproducible-builds/diffoscope/commit/9c0cfdf5a59808b6698eee99600b46cada3c762d
Regards,
--
,''`.
: :' : Chris Lamb
`. `'` la...@debian.org / chris-lamb.co.uk
`-
___
Reproducible-builds mailing list
Reproducible-builds@alioth-lists.debian.net
https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/reproducible-builds
Bug#901758: diffoscope: leaves progress bar on terminal after completion
On Fri, 06 Jul 2018 18:50:44 +0100 Chris Lamb wrote:
> https://salsa.debian.org/reproducible-builds/diffoscope/commit/68c8263a430aca779b0c44fd31e729ae9bd59e0b.patch
> +# Clear the progress bar after completion (#901758)
I don't think mentioning the bug in the comment is useful here,
since the comment is self explanatory without the bug link.
> +self.fd.write("\033[K")
This isn't going to work if the terminal being used doesn't support
that escape sequence or if the user requested a progress bar but
directed output to a file. I'd suggest the approach I mentioned in the
initial bug report instead. Detect the erase-to-eol escape sequence
for the current terminal, use spaces if the current terminal doesn't
support it and don't print anything if the output is not a terminal.
--
bye,
pabs
https://wiki.debian.org/PaulWise
signature.asc
Description: This is a digitally signed message part
___
Reproducible-builds mailing list
Reproducible-builds@alioth-lists.debian.net
https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/reproducible-builds
Bug#901758: diffoscope: leaves progress bar on terminal after completion
Package: diffoscope
Version: 95
Severity: minor
diffoscope leaves a giant progress bar on the terminal after it
completes. It would be nice if the progress bar could disappear after
it has served its purpose. In case the progress bar code you use
doesn't support this, I've included code below from another project
that clears to the end of the line if the terminal supports that and
falls back to overwriting the line with spaces if it does not.
$ touch foo bar
$ diffoscope foo bar
|#|
100% Time: 0:00:00
try:
from shutil import get_terminal_size
def get_columns():
return get_terminal_size().columns
except ImportError:
# Python 3.2 compatibility:
from fcntl import ioctl
from termios import TIOCGWINSZ
from struct import unpack
def get_columns():
try:
buf = ioctl(sys.stdout.fileno(), TIOCGWINSZ, ' ' * 4)
return unpack('hh', buf)[1]
except IOError:
return 80
if sys.stdout.isatty():
from curses import tigetstr, setupterm
setupterm()
erase_line = tigetstr('el')
def erase_to_eol_cr(size=0):
if erase_line:
sys.stdout.buffer.write(erase_line)
else:
width = get_columns()
print(' ' * (width - size), end='')
print(end='\r')
sys.stdout.flush()
-- System Information:
Debian Release: buster/sid
APT prefers testing-debug
APT policy: (900, 'testing-debug'), (900, 'testing'), (800,
'unstable-debug'), (800, 'unstable'), (790, 'buildd-unstable'), (700,
'experimental-debug'), (700, 'experimental'), (690, 'buildd-experimental')
Architecture: amd64 (x86_64)
Kernel: Linux 4.16.0-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_AU.UTF-8, LC_CTYPE=en_AU.UTF-8 (charmap=UTF-8),
LANGUAGE=en_AU:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
Versions of packages diffoscope depends on:
ii libpython3.6-stdlib3.6.6~rc1-1
ii python33.6.5-3
ii python3-distro 1.0.1-2
ii python3-distutils 3.6.5-4
ii python3-libarchive-c 2.1-3.1
ii python3-magic 2:0.4.15-1
ii python3-pkg-resources 39.1.0-1
Versions of packages diffoscope recommends:
ii abootimg 0.6-1+b2
ii acl 2.2.52-3+b1
ii apktool 2.3.3-1
ii binutils-multiarch 2.30-21
ii bzip21.0.6-8.1
ii caca-utils 0.99.beta19-2+b3
ii colord 1.3.3-2
ii db-util 5.3.1
ii default-jdk [java-sdk] 2:1.10-67
ii default-jdk-headless 2:1.10-67
pn device-tree-compiler
pn docx2txt
ii e2fsprogs1.44.2-1
ii enjarify 1:1.0.3-4
ii fontforge-extras 0.3-4
pn fp-utils
ii genisoimage 9:1.1.11-3+b2
ii gettext 0.19.8.1-6+b1
ii ghc 8.0.2-11
ii ghostscript 9.22~dfsg-2.1
ii giflib-tools 5.1.4-3
ii gnumeric 1.12.39-1
ii gnupg2.2.8-1
ii imagemagick 8:6.9.9.39+dfsg-1
ii imagemagick-6.q16 [imagemagick] 8:6.9.9.39+dfsg-1
ii jsbeautifier 1.6.4-7
pn libarchive-tools
ii llvm 1:4.0-40
pn mono-utils
pn odt2txt
pn oggvideotools
ii openjdk-10-jdk [java-sdk]10.0.1+10-4
ii openssh-client 1:7.7p1-2
ii pdftk2.02-4+b2
ii pgpdump 0.31-0.2
ii poppler-utils0.63.0-2
pn procyon-decompiler
ii python3-argcomplete 1.8.1-1
ii python3-binwalk 2.1.1-16
ii python3-debian 0.1.32
pn python3-defusedxml
pn python3-guestfs
ii python3-jsondiff 1.1.1-2
ii python3-progressbar 2.3-4
ii python3-pyxattr 0.6.0-2+b1
ii python3-tlsh 3.4.4+20151206-1+b3
pn r-base-core
ii rpm2cpio 4.14.1+dfsg1-3
ii sng 1.1.0-1+b1
ii sqlite3 3.24.0-1
ii squashfs-tools 1:4.3-6
ii tcpdump 4.9.2-3
ii unzip6.0-21
ii vim-common 2:8.0.1766-1
pn xmlutils
ii xxd 2:8.0.1766-1
ii xz-utils 5.2.2-1.3
Versions of packages diffoscope suggests:
ii libjs-jquery 3.2.1-1
