Simplify format string handling

Instead of using a format that is stored in a three-wide tuple in each
SubredditPage, the displaying is done at display-time. Functionality
related to creating the format has been removed. Additionally, this
simplification makes it possible for correctly spacing the %F format
specifier. Previously, it couldn't easily look ahead to check if a space
was necessary; now, the spacing should be always be correct for any
combination of flair-like information and consecutive spaces will not be
printed to ensure the format isn't made to look strange if a piece of
data is missing.

Tests have also been updated to reflect changes in the SubredditPage
class. The SubredditPage._create_format test has been removed, and much
of its functionality is now tested in the test for
SubredditPage._draw_item_format.

Related to #3
This commit is contained in:
John Helmert
2019-08-08 22:22:26 -05:00
parent 423ef5bd84
commit 446f50be28
3 changed files with 272 additions and 296 deletions

View File

@@ -3,6 +3,7 @@ from __future__ import unicode_literals
import curses
from collections import OrderedDict
import sys
import six
import pytest
@@ -61,23 +62,6 @@ def test_subreddit_page_construct_default(reddit, terminal, config, oauth):
page.draw()
def test_subreddit_page_construct_nondefault(reddit, terminal, config, oauth):
# We're just testing the constructor, and only a little bit of the
# constructor is dependent on a nondefault look and feel, so we can test
# just that little bit. The rest is tested in the above test.
config['look_and_feel'] = 'compact'
with mock.patch.object(SubredditPage, '_create_format'):
with terminal.loader():
page = SubredditPage(reddit, terminal, config, oauth, '/r/python')
SubredditPage._create_format.assert_called_with(config.COMPACT_FORMAT)
config['subreddit_format'] = 'NonNull'
with terminal.loader():
page = SubredditPage(reddit, terminal, config, oauth, '/r/python')
SubredditPage._create_format.assert_called_with('NonNull')
def test_subreddit_refresh(subreddit_page, terminal):
# Refresh the page with default values
@@ -722,17 +706,14 @@ def test_subreddit_page__gold_str(config, terminal, subreddit_page):
data['gold'] = 1
goldstr = subreddit_page._gold_str(data)
assert goldstr == terminal.gilded + ' '
assert goldstr == terminal.gilded
data['gold'] = 2
goldstr = subreddit_page._gold_str(data)
assert goldstr == terminal.gilded + 'x2'
def test_subreddit_page__create_format(terminal, subreddit_page):
# We won't test unimplemented format specifiers. We can add tests when they
# are implemented. We also won't test the lambdas that call another
# function - we test those functions individually
def test_subreddit_page__draw_item_format(terminal, subreddit_page):
data = {
'index': '1',
'title': 'Without saying what the category is, what are your top five?',
@@ -746,6 +727,7 @@ def test_subreddit_page__create_format(terminal, subreddit_page):
'url': 'self.AskReddit',
'url_type': 'selfpost',
'url_full': 'https://www.reddit.com/r/AskReddit/comments/99eh6b/without_saying_what_the_category_is_what_are_your/',
'gold': 1,
'saved': True,
'hidden': True,
'stickied': True,
@@ -757,6 +739,7 @@ def test_subreddit_page__create_format(terminal, subreddit_page):
'%i': '1',
'%t': 'Without saying what the category is, what are your top five?',
'%s': '144655',
'%v': '',
'%c': '26584',
'%r': '10month',
'%e': '(edit 5month)',
@@ -767,157 +750,94 @@ def test_subreddit_page__create_format(terminal, subreddit_page):
'%A': '[saved]',
'%h': '[hidden]',
'%T': '[stickied]',
'%g': '',
'%n': 'NSFW',
'%f': 'Serious Replies Only',
}
# This is what terminal.attr is expected to be called with for each item
expected_attr = {
'%s': 'Score',
'%c': 'CommentCount',
'%r': 'Created',
'%e': 'Created',
'%a': 'SubmissionAuthor',
'%S': 'SubmissionSubreddit',
'%A': 'Saved',
'%h': 'Hidden',
'%T': 'Stickied',
'%g': 'Gold',
'%n': 'NSFW',
'%f': 'SubmissionFlair'
'%i': terminal.attr('SubmissionTitle'),
'%t': terminal.attr('SubmissionTitle'),
'%s': terminal.attr('Score'),
'%v': terminal.attr('Upvote'),
'%c': terminal.attr('CommentCount'),
'%r': terminal.attr('Created'),
'%e': terminal.attr('Created'),
'%a': terminal.attr('SubmissionAuthor'),
'%S': terminal.attr('SubmissionSubreddit'),
'%u': terminal.attr('Link'),
'%U': terminal.attr('Link'),
'%A': terminal.attr('Saved'),
'%h': terminal.attr('Hidden'),
'%T': terminal.attr('Stickied'),
'%g': terminal.attr('Gold'),
'%n': terminal.attr('NSFW'),
'%f': terminal.attr('SubmissionFlair')
}
# This set of tests checks for proper splitting and newline handling
# For simplicity's sake, use explicit newlines
test_format = 'a\n<>'
format_list = None
format_list = subreddit_page._create_format(test_format)
# This gets a bit ugly due to {re|str}.split() adding few null strings.
assert len(format_list) == 7
# %i, \n, <, >
assert format_list[0][0] == 'a'
assert format_list[1][0] == '\n'
assert format_list[3][0] == '<'
assert format_list[5][0] == '>'
# Check for newline flag
assert format_list[2][2] == True
terminal.get_arrow = mock.Mock(return_value=['val1', 'val2'])
terminal.attr = mock.Mock()
subreddit_page._gold_str = mock.Mock()
# This set of tests makes sure each format specifier works as intended
for fmt in filter(None, '%i %t %s %v %c %r %e %a %S %u %U %A %h %T %g %n %f %F'.split()):
format_list = subreddit_page._create_format(fmt)
# First, some special cases
if fmt == '%v':
# User's comment status - upvoted, downvoted, neither
format_list[1][0](data)
# data['likes'] == True
terminal.get_arrow.assert_called_with(True)
elif fmt == '%i' or fmt == '%t' or fmt == '%u' or fmt == '%U':
# Don't check the attribute for these - they have their own
# attribute functions
assert format_list[1][0](data) == expected_str[fmt]
elif fmt == '%g':
# Gold. The string is given by the function _gold_str() which is
# tested elsewhere, so here we just check that it was called
assert subreddit_page._gold_str.assert_called
subreddit_page._gold_str.mock_reset()
elif fmt == '%F':
# The flair catch-all. Check for each individual flair
assert format_list[1][0](data) == expected_str['%f'] + ' '
format_list[1][1](data)
terminal.attr.assert_called_with(expected_attr['%f'])
terminal.attr.reset_mock()
assert format_list[2][0](data) == expected_str['%A'] + ' '
format_list[2][1](data)
terminal.attr.assert_called_with(expected_attr['%A'])
terminal.attr.reset_mock()
assert format_list[3][0](data) == expected_str['%h'] + ' '
format_list[3][1](data)
terminal.attr.assert_called_with(expected_attr['%h'])
terminal.attr.reset_mock()
assert format_list[4][0](data) == expected_str['%T'] + ' '
format_list[4][1](data)
terminal.attr.assert_called_with(expected_attr['%T'])
terminal.attr.reset_mock()
subreddit_page._gold_str.assert_called
assert format_list[6][0](data) == expected_str['%n'] + ' '
else:
# General case
assert format_list[1][0](data) == expected_str[fmt], \
"On specifier {0}".format(fmt)
format_list[1][1](data)
terminal.attr.assert_called_with(expected_attr[fmt])
terminal.attr.reset_mock()
def test_subreddit_page__draw_item_format(terminal, subreddit_page):
win = terminal.stdscr.subwin
terminal.add_line = mock.Mock()
terminal.add_space = mock.Mock()
with mock.patch.object(terminal, 'add_line'):
# Loop through each individual format specifier and check for the desired behavior.
for fmt in filter(None, '%i %t %s %v %c %r %e %a %S %u %U %A %h %T %g %n %f %F'.split()):
subreddit_page.config['subreddit_format'] = fmt
subreddit_page._draw_item_format(win, data, 0, 0)
# Test proper handling of lambdas and not-lambdas
subreddit_page.format = [(lambda data: "string", lambda data: None, False)]
subreddit_page._draw_item_format(win, None, None, 0)
try:
if fmt == '%F':
terminal.add_line.assert_called_with(win, "string", 0, attr=None)
terminal.add_line.reset_mock()
# Make sure each of the things that %F should print are printed
# flair
terminal.add_line.assert_any_call(win, expected_str['%f'],
row=mock.ANY, col=mock.ANY,
attr=expected_attr['%f'])
subreddit_page.format = [("string", lambda data: None, False)]
subreddit_page._draw_item_format(win, None, None, 0)
# saved
terminal.add_line.assert_any_call(win, expected_str['%A'],
row=mock.ANY, col=mock.ANY,
attr=expected_attr['%A'])
terminal.add_line.assert_called_with(win, "string", 0, attr=None)
terminal.add_line.reset_mock()
# hidden
terminal.add_line.assert_any_call(win, expected_str['%h'],
row=mock.ANY, col=mock.ANY,
attr=expected_attr['%h'])
# Test newline handling, shouldn't be drawn
subreddit_page.format = [("\n", lambda data: None, False)]
subreddit_page._draw_item_format(win, None, None, 0)
# stickied
terminal.add_line.assert_any_call(win, expected_str['%T'],
row=mock.ANY, col=mock.ANY,
attr=expected_attr['%T'])
assert not terminal.add_line.called
terminal.add_line.reset_mock()
# gold
terminal.add_line.assert_any_call(win, expected_str['%g'],
row=mock.ANY, col=mock.ANY,
attr=expected_attr['%g'])
# add_line shouldn't be called if the string field is none
subreddit_page.format = [(None, lambda data: None, False)]
subreddit_page._draw_item_format(win, None, None, 0)
assert not terminal.add_line.called
# Check for newline handling and attribute reusing in the call of a null
# attribute
subreddit_page.format = [("string", lambda data: terminal.attr("Link"), True),
("str", None, False)]
subreddit_page._draw_item_format(win, None, None, 0)
# Should be called with a column of 1
terminal.add_line.assert_any_call(win, "string", 0, 1,
attr=terminal.attr("Link"))
# "str" shouldn't be printed at the beginning of the line and it should be
# printed with the same attribute as the previous item
terminal.add_line.assert_any_call(win, "str", 0,
attr=terminal.attr("Link"))
# nsfw
terminal.add_line.assert_any_call(win, expected_str['%n'],
row=mock.ANY, col=mock.ANY,
attr=expected_attr['%n'])
else:
terminal.add_line.assert_called_with(win, expected_str[fmt],
row=0, col=1,
attr=expected_attr[fmt])
except:
sys.stderr.write("Failed on {0}\n".format(fmt))
raise
terminal.add_line.reset_mock()
# Check that spaces are printed with add_space and not add_line
subreddit_page.format = [(" ", lambda data: None, False)]
subreddit_page._draw_item_format(win, None, None, 0)
# Ensure spaces aren't printed consecutively if data is absent
data['gold'] = 0
subreddit_page.config['subreddit_format'] = ' %g '
subreddit_page._draw_item_format(win, data, 0, 0)
assert terminal.add_space.called
assert not terminal.add_line.called
assert terminal.add_line.call_count == 1
terminal.add_line.reset_mock()
# Test for correct handling of separators
subreddit_page.config['subreddit_format'] = ' | '
subreddit_page._draw_item_format(win, data, 0, 0)
# Should be called thrice - ' ', '|', ' '
assert terminal.add_line.call_count == 3

View File

@@ -116,6 +116,17 @@ def main():
# Add an empty handler so the logger doesn't complain
logging.root.addHandler(logging.NullHandler())
if config['subreddit_format']:
# If the user has explicitly set a subreddit format, we don't want to
# try to do something else with the format so we fall out of the if now
pass
elif not config['look_and_feel'] or config['look_and_feel'] == 'default':
# FIXME - default needs its own subreddit_format but can't because
# no wrapped-title specifier exists
pass
elif config['look_and_feel'] == 'compact':
config['subreddit_format'] = config.COMPACT_FORMAT
# Make sure the locale is UTF-8 for unicode support
default_locale = locale.setlocale(locale.LC_ALL, '')
try:

View File

@@ -42,11 +42,6 @@ class SubredditPage(Page):
self.nav = Navigator(self.content.get)
self.toggled_subreddit = None
if self.config['subreddit_format']:
self.format = self._create_format(self.config['subreddit_format'])
elif self.config['look_and_feel'] == 'compact':
self.format = self._create_format(self.config.COMPACT_FORMAT)
def handle_selected_page(self):
"""
Open all selected pages in subwindows except other subreddit pages.
@@ -276,185 +271,234 @@ class SubredditPage(Page):
if data['gold'] > 1:
return self.term.gilded + 'x{}'.format(data['gold'])
elif data['gold'] == 1:
return self.term.gilded + ' '
return self.term.gilded
else:
return ''
def _create_format(self, format_string):
"""
Returns a list of tuples of the format (datafield, attr, first) which
will be used by _draw_item to output information in the proper order.
It would be trivial to use the strings as simple format strings, but
more must be done in order to associate strings with their Attribute.
datafield = lambda that retrieves the proper field of the data
dict
attr = lambda that returns the proper attribute class associated with
datafield. Sometimes this isn't known until drawtime; see above
*_attr() functions for examples
first = boolean that signifies whether or not term.add_line should be
called with a col argument to start a line
"""
form = []
def _draw_item_format(self, win, data, valid_rows, offset):
first = True
# Split the list between %., newlines, and separator characters to
# treat them separately
format_list = re.split(r'(%.|[\n' + re.escape(self.FORMAT_SEP) + '])', format_string, re.DOTALL)
format_list = re.split(r'(%.|[\n' + re.escape(self.FORMAT_SEP) + '])', self.config['subreddit_format'], re.DOTALL)
# Clean the list of null items. We don't need to join this list
# together again, so this is safe.
# https://stackoverflow.com/q/2197451
format_list = [item for item in format_list if item != '']
# Remember whether or not the last character printed was a space. If it
# was, printing more spaces should be avoided.
last_was_space = False
for item in format_list:
# Use lambdas because the actual data to be used is only known at
# drawtime. This way the format list knows how to use the data,
# and can simply be used when the data is available
col = 1 if first else None
# We don't want to print consecutive spaces, so check if a space
# was the last character printed, and skip this iteration if a
# space is to be printed
if last_was_space and item == ' ':
# coverage reports this as never executing, despite
# test_subreddit_page__draw_item_format testing for it, and pdb
# confirming the line is executed
continue
last_was_space = True
if item == "%i":
form.append((lambda data: str(data['index']),
lambda data: self._submission_attr(data), first))
self.term.add_line(win, str(data['index']),
row=offset, col=col,
attr=self._submission_attr(data))
last_was_space = False
elif item == "%t":
form.append((lambda data: data['title'],
lambda data: self._submission_attr(data), first))
self.term.add_line(win, data['title'],
row=offset, col=col,
attr=self._submission_attr(data))
last_was_space = False
elif item == "%s":
# Need to cut off the characters that aren't the score number
form.append((lambda data: str(data['score']),
lambda data: self.term.attr('Score'), first))
self.term.add_line(win, str(data['score']),
row=offset, col=col,
attr=self.term.attr('Score'))
last_was_space = False
elif item == "%v":
# This isn't great, self.term.get_arrow gets called twice
form.append((lambda data: self.term.get_arrow(data['likes'])[0],
lambda data: self.term.get_arrow(data['likes'])[1], first))
arrow = self.term.get_arrow(data['likes'])
self.term.add_line(win, arrow[0],
row=offset, col=col,
attr=arrow[1])
last_was_space = False
elif item == "%c":
form.append((
lambda data: data['comments']
if data['comments'] else None, # Don't try to subscript null items
lambda data: self.term.attr('CommentCount'),
first))
self.term.add_line(win, data['comments'],
row=offset, col=col,
attr=self.term.attr('CommentCount'))
last_was_space = False
elif item == "%r":
form.append((lambda data: data['created'],
lambda data: self.term.attr('Created'), first))
self.term.add_line(win, data['created'],
row=offset, col=col,
attr=self.term.attr('Created'))
last_was_space = False
elif item == "%R":
form.append((lambda data: data['created_exact'],
lambda data: self.term.attr('Created'), first))
self.term.add_line(win, data['created_exact'],
row=offset, col=col,
attr=self.term.attr('Created'))
last_was_space = False
elif item == "%e":
form.append((lambda data: data['edited'],
lambda data: self.term.attr('Created'), first))
self.term.add_line(win, data['edited'],
row=offset, col=col,
attr=self.term.attr('Created'))
last_was_space = False
elif item == "%E":
form.append((lambda data: data['edited_exact'],
lambda data: self.term.attr('Created'), first))
self.term.add_line(win, data['edited_exact'],
row=offset, col=col,
attr=self.term.attr('Created'))
last_was_space = False
elif item == "%a":
form.append((lambda data: data['author'],
lambda data: self.term.attr('SubmissionAuthor'), first))
self.term.add_line(win, data['author'],
row=offset, col=col,
attr=self.term.attr('SubmissionAuthor'))
last_was_space = False
elif item == "%S":
form.append((lambda data: "/r/" + data['subreddit'],
lambda data: self.term.attr('SubmissionSubreddit'),
first))
self.term.add_line(win, "/r/" + data['subreddit'],
row=offset, col=col,
attr=self.term.attr('SubmissionSubreddit'))
last_was_space = False
elif item == "%u":
form.append((lambda data: self._url_str(data),
lambda data: self._url_attr(data), first))
self.term.add_line(win, self._url_str(data),
row=offset, col=col,
attr=self._url_attr(data))
last_was_space = False
elif item == "%U":
form.append((lambda data: data['url'],
lambda data: self._url_attr(data), first))
self.term.add_line(win, data['url'],
row=offset, col=col,
attr=self._url_attr(data))
last_was_space = False
elif item == "%A":
form.append((lambda data: '[saved]' if data['saved'] else '',
lambda data: self.term.attr('Saved'), first))
if data['saved']:
self.term.add_line(win, '[saved]',
row=offset, col=col,
attr=self.term.attr('Saved'))
last_was_space = False
elif item == "%h":
form.append((lambda data: '[hidden]' if data['hidden'] else '',
lambda data: self.term.attr('Hidden'), first))
if data['hidden']:
self.term.add_line(win, '[hidden]',
row=offset, col=col,
attr=self.term.attr('Hidden'))
last_was_space = False
elif item == "%T":
form.append((lambda data: '[stickied]' if data['stickied'] else '',
lambda data: self.term.attr('Stickied'), first))
if data['stickied']:
self.term.add_line(win, '[stickied]',
row=offset, col=col,
attr=self.term.attr('Stickied'))
last_was_space = False
elif item == "%g":
form.append((lambda data: self._gold_str(data),
lambda data: self.term.attr('Gold'), first))
if data['gold'] > 0:
self.term.add_line(win, self._gold_str(data),
row=offset, col=col,
attr=self.term.attr('Gold'))
last_was_space = False
elif item == "%n":
form.append((lambda data: 'NSFW' if data['nsfw'] else '',
lambda data: self.term.attr('NSFW'), first))
if data['nsfw']:
self.term.add_line(win, 'NSFW',
row=offset, col=col,
attr=self.term.attr('NSFW'))
last_was_space = False
elif item == "%f":
form.append((lambda data: data['flair'] if data['flair'] else '',
lambda data: self.term.attr('SubmissionFlair'), first))
if data['flair']:
self.term.add_line(win, data['flair'],
row=offset, col=col,
attr=self.term.attr('SubmissionFlair'))
last_was_space = False
elif item == "%F":
form.append((lambda data: data['flair'] + ' ' if data['flair'] else '',
lambda data: self.term.attr('SubmissionFlair'), first))
if data['flair']:
self.term.add_line(win, data['flair'],
row=offset, col=col,
attr=self.term.attr('SubmissionFlair'))
form.append((lambda data: '[saved] ' if data['saved'] else '',
lambda data: self.term.attr('Saved'), first))
# These ugly if's write a space if necessary. It is
# necessary if there are more flairlike strings to write.
if (data['saved'] or data['hidden'] or
data['stickied'] or data['gold'] or
data['nsfw']):
self.term.add_space(win)
form.append((lambda data: '[hidden] ' if data['hidden'] else '',
lambda data: self.term.attr('Hidden'), first))
last_was_space = False
form.append((lambda data: '[stickied] ' if data['stickied'] else '',
lambda data: self.term.attr('Stickied'), first))
if data['saved']:
self.term.add_line(win, '[saved]',
row=offset, col=col,
attr=self.term.attr('Saved'))
form.append((lambda data: self._gold_str(data),
lambda data: self.term.attr('Gold'), first))
if (data['hidden'] or data['stickied'] or
data['gold'] or data['nsfw']):
self.term.add_space(win)
form.append((lambda data: 'NSFW ' if data['nsfw'] else '',
lambda data: self.term.attr('NSFW'), first))
last_was_space = False
if data['hidden']:
self.term.add_line(win, '[hidden]',
row=offset, col=col,
attr=self.term.attr('Hidden'))
if (data['stickied'] or data['gold'] or
data['nsfw']):
self.term.add_space(win)
last_was_space = False
if data['stickied']:
self.term.add_line(win, '[stickied]',
row=offset, col=col,
attr=self.term.attr('Stickied'))
if data['gold'] or data['nsfw']:
self.term.add_space(win)
last_was_space = False
if data['gold']:
self.term.add_line(win, self._gold_str(data),
row=offset, col=col,
attr=self.term.attr('Gold'))
if data['nsfw']:
self.term.add_space(win)
last_was_space = False
if data['nsfw']:
self.term.add_line(win, 'NSFW',
row=offset, col=col,
attr=self.term.attr('NSFW'))
last_was_space = False
elif item == "\n":
form.append((item, None, first))
first = True
offset += 1
continue
else: # Write something else that isn't in the data dict
# Make certain "separator" characters use the Separator
# attribute
if item in self.FORMAT_SEP:
form.append((item,
lambda data: self.term.attr('Separator'),
first))
self.term.add_line(win, item,
row=offset, col=col,
attr=self.term.attr('Separator'))
else:
form.append((item, None, first))
self.term.add_line(win, item,
row=offset, col=col,
attr=None)
if item != ' ':
last_was_space = False
first = False
return form
def _draw_item_format(self, win, data, valid_rows, offset):
last_attr = None
for get_data, get_attr, first in self.format:
# add_line wants strings, make sure we give it strings
if callable(get_data):
print_data = get_data(data)
else:
# Start writing to the next line if we hit a newline
if get_data == "\n":
offset += 1
continue
# Otherwise, proceed on the same line
print_data = get_data
# Don't try to write None strings to the screen. This happens in
# places like user pages, where data['comments'] is None
if not print_data and first is False:
continue
# We only want to print a maximum of one line of data,
# and we want to cast to a string, since sometimes 'data' is not a
# string, but a Redditor object
# TODO - support line wrapping
string = str(print_data).split('\n')[0]
if string == ' ':
# Make sure spaces aren't treated like normal strings and print
# them to the window this way. This ensures they won't be drawn
# with an attribute.
self.term.add_space(win)
continue
# To make sure we don't try to write a None as an attribute,
# we can use the one that was last used
if get_attr is None:
attr = last_attr
else:
attr = get_attr(data)
if first:
self.term.add_line(win, string, offset, 1, attr=attr)
else:
self.term.add_line(win, string, offset, attr=attr)
last_attr = attr
def _draw_item_default(self, win, data, n_rows, n_cols, valid_rows, offset):
"""
Draw items with default look and feel
@@ -554,8 +598,9 @@ class SubredditPage(Page):
valid_rows = range(0, n_rows)
offset = 0 if not inverted else - (data['n_rows'] - n_rows)
if self.config['look_and_feel'] == 'compact' or \
self.config['subreddit_format']:
# FIXME - this assumes default doesn't have a subreddit_format. In the
# future it should, and when it does it will break this if
if self.config['subreddit_format']:
self._draw_item_format(win, data, valid_rows, offset)
else:
self._draw_item_default(win, data, n_rows, n_cols, valid_rows, offset)