On Sun, Jun 13, 2021 at 10:06:27AM -0700, Kevin J. McCarthy wrote:
> On Sun, Jun 13, 2021 at 03:56:58PM +0200, Rene Kita wrote:
> > Using skip-quoted the first unquoted line becomes the new top line
> > displayed in the pager. This leaves the user with no context to know
> > what the answer refers to.
>
> I need to get through a backlog of other things before I look too closely at
> this. However, I'd like others' opinions on the usefulness of the option.
> My initial impression is that it's not generally useful (i.e., if you need
> context then don't <skip-quoted>) but I'd appreciate others take.
My reason to write this patch was the following:
When skimming messages on mailing lists, especially ones containing
patches I often have to hit <next-page> multiple times to get to the
next unquoted line. When I use <skip-quoted> I don't have context.
With this option I get a little bit of context and can decide if I
want/need to go back more lines. Without this option I always have to go
back a few lines to get the context.
I changed the commit message to make it more clear why I think this
option is useful.
>
> Just glancing quickly, please watch the coding style. e.g.
> while (test)
> {
> }
> not
> while (test){
> }
Fixed.
> > + while (!err && SkipQuotedContext > 0
>
> It looks like the patch disables <skip-quoted> if $skip_quoted_context is
> left at 0?
Yep, this is a bug. This should have been '>= 0'. Fixed.
I only added this check to prevent the user from setting 'SkipQuotedContext'
to a negativ value. Are there better ways to make sure that the option
is set to a valid value?
> > + if (new_topline - context < 0)
> > + context += new_topline - context;
>
> Could that just be:
> if (context > new_topline)
> context = new_topline;
Yes, this is definitely the clearer way to write it.
> Lastly, I'd prefer the config var have "pager_" in front to give it context.
> (Pun intended). e.e. $pager_skip_quoted_context.
Fixed.
I'll wait a few days till I send a v2 to give others the chance to add
their feedback.
> --
> Kevin J. McCarthy
> GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
