From 9e96d36df55fb468c56c8070b9d437c59274e302 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 2 Jul 2017 08:51:57 +0200 Subject: [PATCH 01/15] output via generator --- changelog.md | 1 + mycli/main.py | 66 +++++++++++++++++++++++----------- test/test_main.py | 91 ++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 134 insertions(+), 24 deletions(-) diff --git a/changelog.md b/changelog.md index 1dbe9115..076d9391 100644 --- a/changelog.md +++ b/changelog.md @@ -12,6 +12,7 @@ Features: * Display all result sets returned by stored procedures (Thanks: [Thomas Roten]). * Add current time to prompt options (Thanks: [Thomas Roten]). * Output status text in a more intuitive way (Thanks: [Thomas Roten]). +* Output via generator. (Thanks: Dick Marinus). Bug Fixes: ---------- diff --git a/mycli/main.py b/mycli/main.py index 3b7c48b6..4dd07657 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -37,10 +37,12 @@ from .config import (write_default_config, get_mylogin_cnf_path, open_mylogin_cnf, read_config_files, str_to_bool) from .key_bindings import mycli_bindings -from .encodingutils import utf8tounicode +from .encodingutils import utf8tounicode, text_type from .lexer import MyCliLexer from .__init__ import __version__ +import itertools + click.disable_unicode_literals_warning = True try: @@ -533,7 +535,7 @@ def one_iteration(document=None): if result_count > 0: self.echo('') try: - self.output('\n'.join(formatted), status) + self.output(formatted, status) except KeyboardInterrupt: pass if special.is_timing_enabled(): @@ -661,8 +663,9 @@ def echo(self, s, **kwargs): self.log_output(s) click.secho(s, **kwargs) - def output_fits_on_screen(self, output, status=None): - """Check if the given output fits on the screen.""" + def get_output_margin(self, status=None): + """Get the output margin (number of rows for the prompt, footer and + timing message.""" size = self.cli.output.get_size() margin = self.get_reserved_space() + self.get_prompt(self.prompt_format).count('\n') + 1 @@ -671,11 +674,8 @@ def output_fits_on_screen(self, output, status=None): if status: margin += 1 + status.count('\n') - for i, line in enumerate(output.splitlines(), 1): - if len(line) > size.columns or i > (size.rows - margin): - return False + return margin - return True def output(self, output, status=None): """Output text to stdout or a pager command. @@ -688,15 +688,39 @@ def output(self, output, status=None): """ if output: - self.log_output(output) - special.write_tee(output) - special.write_once(output) - - if (self.explicit_pager or - (special.is_pager_enabled() and not self.output_fits_on_screen(output, status))): - click.echo_via_pager(output) - else: - click.secho(output) + size = self.cli.output.get_size() + + margin = self.get_output_margin(status) + + fits = True + buf = [] + output_via_pager = self.explicit_pager and special.is_pager_enabled() + for i, line in enumerate(output, 1): + self.log_output(line) + special.write_tee(line) + special.write_once(line) + + if fits or output_via_pager: + # buffering + buf.append(line) + + if len(line) > size.columns or i > (size.rows - margin): + # doesn't fit, flush buffer + fits = False + if special.is_pager_enabled: + output_via_pager = True + if not output_via_pager: + for line in buf: + click.secho(line) + buf = [] + + if buf: + if output_via_pager: + # sadly click.echo_via_pager doesn't accept generators + click.echo_via_pager("\n".join(buf)) + else: + for line in buf: + click.secho(line) if status: self.log_output(status) @@ -787,7 +811,7 @@ def format_output(self, title, cur, headers, expanded=False, output = [] if title: # Only print the title if it's not None. - output.append(title) + output = itertools.chain(output, [title]) if cur: rows = list(cur) @@ -799,7 +823,10 @@ def format_output(self, title, cur, headers, expanded=False, formatted = self.formatter.format_output( rows, headers, format_name='vertical') - output.append(formatted) + if isinstance(formatted, (str, text_type)): + formatted = formatted.splitlines() + output = itertools.chain(output, formatted) + return output @@ -951,7 +978,6 @@ def cli(database, user, host, port, socket, password, dbname, if csv: mycli.formatter.format_name = 'csv' - new_line = False elif not table: mycli.formatter.format_name = 'tsv' diff --git a/test/test_main.py b/test/test_main.py index 30f9374f..d7130937 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -10,6 +10,7 @@ from utils import USER, HOST, PORT, PASSWORD, dbtest, run from textwrap import dedent +from collections import namedtuple try: text_type = basestring @@ -64,10 +65,10 @@ def test_execute_arg_with_csv(executor): sql = 'select * from test;' runner = CliRunner() result = runner.invoke(cli, args=CLI_ARGS + ['-e', sql] + ['--csv']) - expected = 'a\nabc\n\n' + expected = 'a\nabc\n' assert result.exit_code == 0 - assert expected in result.output + assert expected in "".join(result.output) @dbtest @@ -84,7 +85,7 @@ def test_batch_mode(executor): result = runner.invoke(cli, args=CLI_ARGS, input=sql) assert result.exit_code == 0 - assert 'count(*)\n3\n\na\nabc\n' in result.output + assert 'count(*)\n3\na\nabc\n' in "".join(result.output) @dbtest @@ -129,7 +130,7 @@ def test_batch_mode_csv(executor): expected = 'a,b\nabc,def\nghi,jkl\n' assert result.exit_code == 0 - assert expected in result.output + assert expected in "".join(result.output) @dbtest @@ -199,3 +200,85 @@ def test_command_descriptions_end_with_periods(): MyCli() for _, command in SPECIAL_COMMANDS.items(): assert command[3].endswith('.') + + +def output(monkeypatch, terminal_size, testdata, explicit_pager, expect_pager): + global clickoutput + clickoutput = "" + m = MyCli() + + class TestOutput(): + def get_size(self): + size = namedtuple('Size', 'rows columns') + size.columns, size.rows = terminal_size + return size + + class TestExecute(): + host = 'test' + user = 'test' + dbname = 'test' + port = 0 + + def server_type(self): + return ['test'] + + class CommandLineInterface(): + output = TestOutput() + + m.cli = CommandLineInterface() + m.sqlexecute = TestExecute() + m.explicit_pager = explicit_pager + + def echo_via_pager(s): + assert expect_pager + global clickoutput + clickoutput += s + + def secho(s): + assert not expect_pager + global clickoutput + clickoutput += s + "\n" + + monkeypatch.setattr(click, 'echo_via_pager', echo_via_pager) + monkeypatch.setattr(click, 'secho', secho) + m.output(testdata) + if clickoutput.endswith("\n"): + clickoutput = clickoutput[:-1] + assert clickoutput == "\n".join(testdata) + + +def test_conditional_pager(monkeypatch): + testdata = "Lorem ipsum dolor sit amet consectetur adipiscing elit sed do".split( + " ") + # User didn't set pager, output doesn't fit screen -> pager + output( + monkeypatch, + terminal_size=(5, 10), + testdata=testdata, + explicit_pager=False, + expect_pager=True + ) + # User didn't set pager, output fits screen -> no pager + output( + monkeypatch, + terminal_size=(20, 20), + testdata=testdata, + explicit_pager=False, + expect_pager=False + ) + # User manually configured pager, output doesn't fit screen -> pager + output( + monkeypatch, + terminal_size=(5, 10), + testdata=testdata, + explicit_pager=True, + expect_pager=True + ) + # User manually configured pager, output fit screen -> pager + output( + monkeypatch, + terminal_size=(20, 20), + testdata=testdata, + explicit_pager=True, + expect_pager=True + ) From e365497ae49f99a66680fba9faf67cba1d088c72 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 9 Jul 2017 13:48:59 +0200 Subject: [PATCH 02/15] is_pager_enabled needs to be a function call --- mycli/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mycli/main.py b/mycli/main.py index f94d50a0..3810570a 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -709,7 +709,7 @@ def output(self, output, status=None): if len(line) > size.columns or i > (size.rows - margin): # doesn't fit, flush buffer fits = False - if special.is_pager_enabled: + if special.is_pager_enabled(): output_via_pager = True if not output_via_pager: for line in buf: From 1aa9e09dcba82991a39797fcd4caaa4c1a3d76b4 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 9 Jul 2017 14:12:28 +0200 Subject: [PATCH 03/15] fixup! output via generator --- mycli/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mycli/main.py b/mycli/main.py index 35cd38c6..62c3ad1e 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -827,7 +827,8 @@ def format_output(self, title, cur, headers, expanded=False, formatted = self.formatter.format_output( rows, headers, format_name='vertical' if expanded else None, **output_kwargs) - first_line = formatted[:formatted.find('\n')] + first_line = next(formatted) + formatted = itertools.chain([first_line], formatted) if (not expanded and max_width and headers and rows and len(first_line) > max_width): From 79629ad2696700377ff3216205fe76945199a03a Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 9 Jul 2017 14:28:41 +0200 Subject: [PATCH 04/15] fixup! output via generator --- mycli/main.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mycli/main.py b/mycli/main.py index 62c3ad1e..7df9eccc 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -827,6 +827,10 @@ def format_output(self, title, cur, headers, expanded=False, formatted = self.formatter.format_output( rows, headers, format_name='vertical' if expanded else None, **output_kwargs) + + if isinstance(formatted, (str, text_type)): + formatted = iter(formatted.splitlines()) + first_line = next(formatted) formatted = itertools.chain([first_line], formatted) @@ -835,8 +839,6 @@ def format_output(self, title, cur, headers, expanded=False, formatted = self.formatter.format_output( rows, headers, format_name='vertical', **output_kwargs) - if isinstance(formatted, (str, text_type)): - formatted = formatted.splitlines() output = itertools.chain(output, formatted) From 585624271e7781bc3877c3ede002577161bc3300 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 9 Jul 2017 14:55:54 +0200 Subject: [PATCH 05/15] fixup! output via generator --- mycli/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mycli/main.py b/mycli/main.py index 7df9eccc..88dfea4d 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -838,6 +838,8 @@ def format_output(self, title, cur, headers, expanded=False, len(first_line) > max_width): formatted = self.formatter.format_output( rows, headers, format_name='vertical', **output_kwargs) + if isinstance(formatted, (str, text_type)): + formatted = iter(formatted.splitlines()) output = itertools.chain(output, formatted) From 53b2c0e40e476ba87ac40884ae3a9cbb13625ccf Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 9 Jul 2017 14:59:56 +0200 Subject: [PATCH 06/15] fixup! output via generator --- test/features/steps/auto_vertical.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/features/steps/auto_vertical.py b/test/features/steps/auto_vertical.py index 41a0af79..eeacaeed 100644 --- a/test/features/steps/auto_vertical.py +++ b/test/features/steps/auto_vertical.py @@ -40,7 +40,7 @@ def step_see_large_results(context): rows = ['{n:3}| {n}'.format(n=str(n)) for n in range(1, 50)] expected = ('***************************[ 1. row ]' '***************************\r\n' + - '{}\r\n\r\n'.format('\r\n'.join(rows))) + '{}\r\n'.format('\r\n'.join(rows))) wrappers.expect_pager(context, expected, timeout=5) wrappers.expect_exact(context, '1 row in set', timeout=2) From 17d60bc52072535667c5731e1f9fb044e80cbd05 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 9 Jul 2017 15:11:16 +0200 Subject: [PATCH 07/15] fixup! output via generator --- mycli/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mycli/main.py b/mycli/main.py index 88dfea4d..57628a1e 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -699,7 +699,8 @@ def output(self, output, status=None): output_via_pager = self.explicit_pager and special.is_pager_enabled() for i, line in enumerate(output, 1): self.log_output(line) - special.write_tee(line) + # line needs to be casted to unicode for click with Python 2.7 + special.write_tee(text_type(line)) special.write_once(line) if fits or output_via_pager: From ea991b40b1a915f4957d1d3a9c78efc0243da6b6 Mon Sep 17 00:00:00 2001 From: Thomas Roten Date: Sun, 9 Jul 2017 14:45:56 -0500 Subject: [PATCH 08/15] Test for disabled pager with no-fit results. --- test/test_main.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/test_main.py b/test/test_main.py index d7130937..6afd7543 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -282,3 +282,13 @@ def test_conditional_pager(monkeypatch): explicit_pager=True, expect_pager=True ) + + SPECIAL_COMMANDS['nopager'].handler() + output( + monkeypatch, + terminal_size=(5, 10), + testdata=testdata, + explicit_pager=False, + expect_pager=False + ) + SPECIAL_COMMANDS['pager'].handler('') From 019aa05e9004a9386a520db1f79b22177a19c126 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Mon, 10 Jul 2017 21:10:30 +0200 Subject: [PATCH 09/15] fixup! output via generator --- mycli/main.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/mycli/main.py b/mycli/main.py index 57628a1e..2e2314e5 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -706,16 +706,19 @@ def output(self, output, status=None): if fits or output_via_pager: # buffering buf.append(line) - - if len(line) > size.columns or i > (size.rows - margin): - # doesn't fit, flush buffer - fits = False - if special.is_pager_enabled(): - output_via_pager = True - if not output_via_pager: - for line in buf: - click.secho(line) - buf = [] + if len(line) > size.columns or i > (size.rows - margin): + fits = False + if not self.explicit_pager and special.is_pager_enabled(): + # doesn't fit, use pager + output_via_pager = True + + if not output_via_pager: + # doesn't fit, flush buffer + for line in buf: + click.secho(line) + buf = [] + else: + click.secho(line) if buf: if output_via_pager: From ba41e21d68d6b81d44d2aeecd60506de145f4bb1 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 6 Aug 2017 12:09:15 +0200 Subject: [PATCH 10/15] fixup! Merge remote-tracking branch 'upstream/master' into feature/output_via_generator --- mycli/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mycli/main.py b/mycli/main.py index ad8440b0..15bbce66 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -863,7 +863,7 @@ def sanitize(col): if (not expanded and max_width and headers and cur and len(first_line) > max_width): formatted = self.formatter.format_output( - rows, headers, format_name='vertical', column_types=column_types, **output_kwargs) + cur, headers, format_name='vertical', column_types=column_types, **output_kwargs) if isinstance(formatted, (str, text_type)): formatted = iter(formatted.splitlines()) From 8c1193dfe6c3fab944055f3ebb6edd906b40bcff Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Sun, 6 Aug 2017 20:26:53 +0200 Subject: [PATCH 11/15] fixup! output via generator --- mycli/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mycli/main.py b/mycli/main.py index 15bbce66..f48b8499 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -855,7 +855,8 @@ def sanitize(col): **output_kwargs) if isinstance(formatted, (str, text_type)): - formatted = iter(formatted.splitlines()) + formatted = formatted.splitlines() + formatted = iter(formatted) first_line = next(formatted) formatted = itertools.chain([first_line], formatted) From 5b836c13d580ebc080ba8bdd9c096dadd01a13f8 Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Mon, 7 Aug 2017 08:54:00 +0200 Subject: [PATCH 12/15] Remove cast to text_type which was required for py27 and fixed by Thomas --- mycli/main.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/mycli/main.py b/mycli/main.py index f48b8499..c84c1263 100755 --- a/mycli/main.py +++ b/mycli/main.py @@ -710,8 +710,7 @@ def output(self, output, status=None): output_via_pager = self.explicit_pager and special.is_pager_enabled() for i, line in enumerate(output, 1): self.log_output(line) - # line needs to be casted to unicode for click with Python 2.7 - special.write_tee(text_type(line)) + special.write_tee(line) special.write_once(line) if fits or output_via_pager: @@ -854,7 +853,7 @@ def sanitize(col): column_types=column_types, **output_kwargs) - if isinstance(formatted, (str, text_type)): + if isinstance(formatted, (text_type)): formatted = formatted.splitlines() formatted = iter(formatted) @@ -865,7 +864,7 @@ def sanitize(col): len(first_line) > max_width): formatted = self.formatter.format_output( cur, headers, format_name='vertical', column_types=column_types, **output_kwargs) - if isinstance(formatted, (str, text_type)): + if isinstance(formatted, (text_type)): formatted = iter(formatted.splitlines()) output = itertools.chain(output, formatted) From 6d0778b104b762c17b2620c53a6f009e79a4cb0b Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Mon, 7 Aug 2017 17:08:12 +0200 Subject: [PATCH 13/15] Update / rephrase changelog --- changelog.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/changelog.md b/changelog.md index be20c676..0d37dd38 100644 --- a/changelog.md +++ b/changelog.md @@ -12,6 +12,11 @@ Internal Changes: ----------------- * Use less memory when formatting results for display (Thanks: [Dick Marinus]). + This is done by passing the database cursor directly to `cli_helpers` and + allow generators for output formats. This also contains a change in the communication + between `cli_helpers` and `mycli`. The output from `cli_helpers` is now formatted as an + iterable with an unicode string for every line instead of a single unicode string with newline + characters. 1.12.0: ======= @@ -36,7 +41,6 @@ Features: * Display all result sets returned by stored procedures (Thanks: [Thomas Roten]). * Add current time to prompt options (Thanks: [Thomas Roten]). * Output status text in a more intuitive way (Thanks: [Thomas Roten]). -* Output via generator. (Thanks: Dick Marinus). * Add colored/styled headers and odd/even rows (Thanks: [Thomas Roten]). * Keyword completion casing (upper/lower/auto) (Thanks: [Irina Truong]). From 4ef958f89ef0e829be50d25f9be2375081d2403b Mon Sep 17 00:00:00 2001 From: Dick Marinus Date: Mon, 7 Aug 2017 18:32:51 +0200 Subject: [PATCH 14/15] fixup! Update / rephrase changelog --- changelog.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog.md b/changelog.md index 0d37dd38..8d0ee23c 100644 --- a/changelog.md +++ b/changelog.md @@ -15,8 +15,8 @@ Internal Changes: This is done by passing the database cursor directly to `cli_helpers` and allow generators for output formats. This also contains a change in the communication between `cli_helpers` and `mycli`. The output from `cli_helpers` is now formatted as an - iterable with an unicode string for every line instead of a single unicode string with newline - characters. + iterable with an unicode string for every line instead of a single unicode string seperated by + newline characters. 1.12.0: ======= From e9b39495c4ea6bb6f89996059acf8e88116db579 Mon Sep 17 00:00:00 2001 From: Thomas Roten Date: Tue, 8 Aug 2017 07:53:33 -0500 Subject: [PATCH 15/15] Simplify changelog. --- changelog.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/changelog.md b/changelog.md index 8d0ee23c..582d85a1 100644 --- a/changelog.md +++ b/changelog.md @@ -12,11 +12,7 @@ Internal Changes: ----------------- * Use less memory when formatting results for display (Thanks: [Dick Marinus]). - This is done by passing the database cursor directly to `cli_helpers` and - allow generators for output formats. This also contains a change in the communication - between `cli_helpers` and `mycli`. The output from `cli_helpers` is now formatted as an - iterable with an unicode string for every line instead of a single unicode string seperated by - newline characters. +* Preliminary work for a future change in outputting results that uses less memory (Thanks: [Dick Marinus]). 1.12.0: =======