On Thu, Aug 04, 2022 at 11:35:31PM +0200, Pavel Sanda wrote:
> On Sun, Jul 31, 2022 at 07:29:35AM -0400, Scott Kostyshak wrote:
> > On Sun, Jul 31, 2022 at 11:34:25AM +0200, Pavel Sanda wrote:
> > > On Sun, Jul 31, 2022 at 05:16:17AM -0400, Scott Kostyshak wrote:
> > > > I'm on vacation and won't be able to test for a few weeks. Can you do
> > > > the test I described before? Just select a big document, and then hold
> > > > Shift + to decrease the selection towards the beginning of the
> > > > document. I just counted using a wrist stopwatch and compared
> > > > with/without the patch.
> > >
> > > I did that already and haven't seen noticeable lag. But I might have too
> > > powerful machine, that's why I ask. Anyway, increasing delay would be
> > > trivial change later.
> > That's good enough for me. 0.5 seems fine to me and I imagine it could
>
> Attached is a newer version. Actually something like 4th rewrite - I figured
> that even the combination of mid-sized document and very fast keyboard repeat
> rates has already visible interference from stats computation.
>
> I ended with more conservative approach - stats updates have now 5s sampling
> rate and I think that's totally fine for casual statusbar look how your
> document is doing.
>
> The only situation when sampling rate get's back 0.5s is when small
> piece (<5000 chars) of text is being selected - that's when you need
> instant visual feedback (and it's the initial motivation for this whole
> patch).
I think it's a nice idea to change the timer based on how many
characters there are.
> I suggest we put this into master and let ppl test on their machines
> if any noticeable changes are visible when editing documents. At worst
> we can disable this feature by default.
Sounds good to me. Thanks for the patch updates. I suggest to commit now.
I have a few comments that are from a quick look of the patch. Please
ignore them if they don't make sense or you disagree. They are really
picky, but I could not help it. Sorry :)
1. I initially thought "chars_blanks" was the number of blank
characters. I think it is worth the longer name to put
"chars_with_blanks" or "chars_w_blanks". I promise I will not mind if
you diagree and keep "chars_blanks".
2. Some consts are defined with descriptions which is helpful for
reading (and for anyone who wants to tweak). There is also a 5000 in
"if (chars_blanks < 5000 && cur.selection())". Would it make sense to
make the 5000 a const variable along with the others?
3. The name of the setting is "Show Document Statistics". I wonder if it
would be helpful to mention something about selection, because when I
see "document" I think "the full document". Would it be too verbose
to have "Show Document (or Selection) Statistics" ? Or perhaps remove
"document", e.g., "Show Statistics".
Scott
signature.asc
Description: PGP signature
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
