New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ceph.in: adjust usage width according to user's tty #15190

Merged
merged 2 commits into from May 23, 2017

Conversation

Projects
None yet
2 participants
@tchaikov
Contributor

tchaikov commented May 21, 2017

fixed a pep8 warning also

Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov tchaikov added the feature label May 21, 2017

@liewegas liewegas requested a review from dmick May 22, 2017

src/ceph.in Outdated
return 80
try:
import shutil
sz = shutil.get_terminal_size((80, 20))

This comment has been minimized.

@dmick

dmick May 22, 2017

Member

Is this Python 3.x?

This comment has been minimized.

@tchaikov

tchaikov May 22, 2017

Contributor

python 3.3 and up.

This comment has been minimized.

@tchaikov

tchaikov May 22, 2017

Contributor

that's why i have a except clause below.

This comment has been minimized.

@dmick
src/ceph.in Outdated
import struct
import termios
arg = struct.pack('HHHH', 0, 0, 0, 0)
ret = fcntl.ioctl(sys.stdout.fileno(), termios.TIOCGWINSZ, arg)

This comment has been minimized.

@dmick

dmick May 22, 2017

Member

We should probably figure out a way to reuse ceph_daemon.Termsize here (maybe moving it out of ceph_daemon)

src/ceph.in Outdated
helplines = [l for l in wrap(cmd['help'], 39, 1)]
width = terminal_width() - 1 # 1 for the line between sig and help
sig_width = int(width / 2)
help_width = int(width / 2) + (width % 2)

This comment has been minimized.

@dmick

dmick May 22, 2017

Member

Hm. The intent of "-1" above was to handle the one-character indent of following lines; not sure that "+ (width % 2)" accomplishes that?

This comment has been minimized.

@tchaikov

tchaikov May 22, 2017

Contributor

to make sure sig_width + help_width == width.

for (s, h) in zip(siglines, helplines):
fullusage += '{0:40s} {1}\n'.format(s, h)
for s, h in zip(siglines, helplines):
fullusage += '{s:{w}s} {h}\n'.format(s=s, h=h, w=sig_width)

This comment has been minimized.

@dmick

dmick May 22, 2017

Member

awesome, I did not know python's string formatting accepted variable width. That's great to know.

tchaikov added some commits May 21, 2017

ceph.in: adjust usage width according to user's tty
fixed a pep8 warning also

Signed-off-by: Kefu Chai <kchai@redhat.com>
pybind/ceph_daemon.py: move _gettermsize() into Termsize
as the latter is the only consumer of _gettermsize(). and a little bit
refactor to improve the readability and be more pep8 compliant.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 23, 2017

@dmick fixed and pushed.

(self.rows != rows) or (self.cols != cols)
)
rows, cols = self._gettermsize()
if not self.changed:

This comment has been minimized.

@dmick

dmick May 23, 2017

Member

You don't like my or? FINE! :)

@dmick

dmick approved these changes May 23, 2017

Looks fine to me.

@dmick dmick merged commit c59cfff into ceph:master May 23, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@tchaikov tchaikov deleted the tchaikov:wip-ceph-term-width branch May 24, 2017

)
rows, cols = self._gettermsize()
if not self.changed:
self.changed = (self.raw,self.col) != (rows, cols)

This comment has been minimized.

@tchaikov

tchaikov May 24, 2017

Contributor

i confess: i just traded your ors with a bug.

This comment has been minimized.

@tchaikov

This comment has been minimized.

@dmick

dmick May 25, 2017

Member

heh. oops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment