Suggested fix lp:#213718 (bazaar pager)

[Joshua Judson Rosen]

Why the contortions to check if `text' is actually a list?

Rather than this:

	if "list" in repr(type(text)):

Shouldn't that just be:

	if isinstance(text, list):

???

Or is there another `list-like' type/class that bzrlib is using here?

--

On 2015-04-11 10:03, Richard Wilbur wrote:
> 
> A comment on bzrlib/ui/text.py:TextUIOutputStream._is_long():
> +    def _is_long(self, text):
> +        if "list" in repr(type(text)):
> +            if len(text) > self.pager_cutoff:
> +                return True
> +            else:
> +                return False
> +        else:
> +            number_of_lines = 0
> +            while "\n" in text:
> +                text = text[text.find("\n")+1:]
> +                number_of_lines = number_of_lines + 1
> +            if number_of_lines > self.pager_cutoff:
> +                return True
> +            else:
> +                return False
> 
> What if we simplify the 'list' case to return the boolean comparison, as in:
> +        if "list" in repr(type(text)):
> +            return len(text) > self.pager_cutoff
> 
> Now, on the string case, I like the fact that we are processing
> incrementally.  We could stop looking as soon as we reach the pager
> threshold.
> +        else:
> +            number_of_lines = 0
> +            start = 0
> +            while (start = text.find("\n", start)+1) > 0:
> +                number_of_lines = number_of_lines + 1
> +                if number_of_lines > self.pager_cutoff:
> +                    return True
> +            return False
> 
> I haven't tested this but string.find returns -1 when it fails.
> 
> I'll be offline most of today till this evening.
> 
> Sincerely,
> Richard
> 

-- 
"Don't be afraid to ask (λf.((λx.xx) (λr.f(rr))))."