Re: [patch] Selection stats in statusbar
On Thu, Aug 18, 2022 at 09:55:50AM +0200, Jürgen Spitzmüller wrote: > showMessage() is triggered on any cursor action. That's way too often. > E.g., it would fire down statistics unnecessarily if you move with the > arrow keys through your document. I'll add just few observational notes collected few weeks back when I was looking into the issue. 1) On the trigger. The nice part of Daniel's patch was that it did not trigger statistics recount at *every* cursor action, most importantly the update did not happen when you start moving with cursor (and hold) but only when you release the movement key. So no update firestorm in such case. If you just move with the cursor slowly via hit on the keyboard the stat recount was not noticeable in my behavioral measurements. I agree it's not conceptually cleanest to pin it to showMessage. OTOH when I tried to look whether we have some natural point of entry in the code where to trigger the update I was not particularly succesfull. Unless the showMessage trick was used it seemed many different place needed touch and most problematic was how to make sure I did not forget about some route how buffer changes. Or prevent future implementation miss (but you might say the same about showMessage, or even worse because no strict policy about its call is in place). 2) On the update execution. If you go by multi-threading route, you enter swampland. The only crash free solution would be to clone the whole buffer and do counting on it, but that does not make sense performace wise, because clone takes asymptotically the same as stats themselves. Otherwise you need to avoid buffer changes while counting stats. Getting this right seemed like recipy for headache and chasing out-of-nowhere crashes - be it from my own overlooking or subtleties/bugs of underlying libraries. So while initially I saw QTimer attachment to the same thread as a drawback, on a second thought we'll get rid of race condition headaches. If we can get the trigger right this should be good enough. I do not have strong opinion about the final solution, just voicing my thoughts when I was playing with the code... Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Freitag, dem 19.08.2022 um 07:06 +0200 schrieb Jürgen Spitzmüller: > The crash is fixed in the attached patch, which works well for me. I have still crashes (when pasting). I don't have time to work on it further for the time being, so I put it on hold. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar[RESOLVED]
Am Fri, 19 Aug 2022 10:47:06 +0200 schrieb Kornel Benko : > Am Fri, 19 Aug 2022 10:44:23 +0200 > schrieb Kornel Benko : > > > Am Fri, 19 Aug 2022 10:16:43 +0200 > > schrieb "Jürgen Spitzmüller" : > > > > > Am Freitag, dem 19.08.2022 um 10:07 +0200 schrieb Kornel Benko: > > > > Both. > > > > > > Interesting. I'd need a recipe for the latter. > > > > > > > Nothing specific. Does not matter what or how I select some parts, > > the number of spaces is wrong (too high). Comparing with the dialog output > > ... > > Number of chars > > in dialogin status bar > > 14 1414 > > 263 263263 > > 136 136136 > > > > > > I'd say, in the status bar the value is output twice. > > > > Kornel > > BUT ONLY if using Slovak GUI. I have to check sk.po. > > Kornel Yes, that was it. Kornel pgpC2odwpd3FX.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Fri, 19 Aug 2022 10:44:23 +0200 schrieb Kornel Benko : > Am Fri, 19 Aug 2022 10:16:43 +0200 > schrieb "Jürgen Spitzmüller" : > > > Am Freitag, dem 19.08.2022 um 10:07 +0200 schrieb Kornel Benko: > > > Both. > > > > Interesting. I'd need a recipe for the latter. > > > > Nothing specific. Does not matter what or how I select some parts, > the number of spaces is wrong (too high). Comparing with the dialog output ... > Number of chars > in dialogin status bar > 14 1414 > 263 263263 > 136 136136 > > > I'd say, in the status bar the value is output twice. > > Kornel BUT ONLY if using Slovak GUI. I have to check sk.po. Kornel pgp8BgAUvfHpl.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Fri, 19 Aug 2022 10:16:43 +0200 schrieb "Jürgen Spitzmüller" : > Am Freitag, dem 19.08.2022 um 10:07 +0200 schrieb Kornel Benko: > > Both. > > Interesting. I'd need a recipe for the latter. > Nothing specific. Does not matter what or how I select some parts, the number of spaces is wrong (too high). Comparing with the dialog output ... Number of chars in dialogin status bar 14 1414 263 263263 136 136136 I'd say, in the status bar the value is output twice. Kornel pgpOoqIU7i0np.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Freitag, dem 19.08.2022 um 10:07 +0200 schrieb Kornel Benko: > Both. Interesting. I'd need a recipe for the latter. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Fri, 19 Aug 2022 09:59:45 +0200 schrieb "Jürgen Spitzmüller" : > Am Freitag, dem 19.08.2022 um 09:54 +0200 schrieb Kornel Benko: > > I seen it in the status bar. Dialog shows OK. > > And only of you select whole words (e.g. via double click)? Or also if > you select manually? > Both. Kornel pgpswgR6txZVT.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Freitag, dem 19.08.2022 um 09:54 +0200 schrieb Kornel Benko: > I seen it in the status bar. Dialog shows OK. And only of you select whole words (e.g. via double click)? Or also if you select manually? -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Fri, 19 Aug 2022 09:34:29 +0200 schrieb "Jürgen Spitzmüller" : > Am Freitag, dem 19.08.2022 um 08:42 +0200 schrieb Kornel Benko: > > There is something not correct with showing number of spaces in > > Chinese docs. > > > > 1.) Open zh_CN/Tutorial.lyx > > 2.) Select a single word e.g. 参考文献和交叉引用有点像 > > => you get > > number of words 1 > > number of char 1212 > > number of char w/o spaces 12 > > > > Same for Russion docs. > > Even German docs shows too many spaces. > > Is this only in the status bar or also in the dialog? > I seen it in the status bar. Dialog shows OK. Kornel pgpwUS6MnkPO0.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Freitag, dem 19.08.2022 um 08:42 +0200 schrieb Kornel Benko: > There is something not correct with showing number of spaces in > Chinese docs. > > 1.) Open zh_CN/Tutorial.lyx > 2.) Select a single word e.g. 参考文献和交叉引用有点像 > => you get > number of words1 > number of char 1212 > number of char w/o spaces 12 > > Same for Russion docs. > Even German docs shows too many spaces. Is this only in the status bar or also in the dialog? -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Fri, 19 Aug 2022 08:42:20 +0200 schrieb Kornel Benko : > Am Fri, 19 Aug 2022 07:06:57 +0200 > schrieb "Jürgen Spitzmüller" : > > > Am Donnerstag, dem 18.08.2022 um 18:43 +0200 schrieb Jean-Marc > > Lasgouttes: > > > Indeed, threads create lots of interesting bugs... It would be better > > > if we could avoid them > > > > The UX difference to the timer approach is huge. The threaded > > calculation feels smooth and is almost instant. > > > > The crash is fixed in the attached patch, which works well for me. > > > > There is something not correct with showing number of spaces in Chinese docs. > > 1.) Open zh_CN/Tutorial.lyx > 2.) Select a single word e.g. 参考文献和交叉引用有点像 > => you get >number of words1 >number of char 1212 >number of char w/o spaces 12 > > Same for Russion docs. > Even German docs shows too many spaces. > Nice work BTW. Kornel pgpezwN6Z8H_3.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Fri, 19 Aug 2022 07:06:57 +0200 schrieb "Jürgen Spitzmüller" : > Am Donnerstag, dem 18.08.2022 um 18:43 +0200 schrieb Jean-Marc > Lasgouttes: > > Indeed, threads create lots of interesting bugs... It would be better > > if we could avoid them > > The UX difference to the timer approach is huge. The threaded > calculation feels smooth and is almost instant. > > The crash is fixed in the attached patch, which works well for me. > There is something not correct with showing number of spaces in Chinese docs. 1.) Open zh_CN/Tutorial.lyx 2.) Select a single word e.g. 参考文献和交叉引用有点像 => you get number of words 1 number of char 1212 number of char w/o spaces 12 Same for Russion docs. Even German docs shows too many spaces. Kornel pgpaHC32M6Xm_.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Donnerstag, dem 18.08.2022 um 18:43 +0200 schrieb Jean-Marc
Lasgouttes:
> Indeed, threads create lots of interesting bugs... It would be better
> if we could avoid them
The UX difference to the timer approach is huge. The threaded
calculation feels smooth and is almost instant.
The crash is fixed in the attached patch, which works well for me.
--
Jürgen
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index da630da90a..88f21ced0e 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -4223,6 +4223,13 @@ void Buffer::structureChanged() const
}
+void Buffer::updateStats(bool threaded) const
+{
+ if (d->gui_)
+ d->gui_->updateStats(threaded);
+}
+
+
void Buffer::errors(string const & err, bool from_master) const
{
if (d->gui_)
@@ -4944,6 +4951,7 @@ void Buffer::updateBuffer() const
{
updateBuffer(UpdateMaster, InternalUpdate);
d->need_update = false;
+ updateStats(false);
}
diff --git a/src/Buffer.h b/src/Buffer.h
index ba066f724c..5ef97828af 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -660,6 +660,8 @@ public:
void updateTocItem(std::string const &, DocIterator const &) const;
/// This function is called when the buffer structure is changed.
void structureChanged() const;
+ /// This function updates the statistics in the stats bar.
+ void updateStats(bool threaded = true) const;
/// This function is called when some parsing error shows up.
void errors(std::string const & err, bool from_master = false) const;
/// This function is called when the buffer busy status change.
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index da309d63e5..85badcf9ae 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -2827,6 +2827,7 @@ bool BufferView::mouseSetCursor(Cursor & cur, bool const select)
d->cursor_.resetAnchor();
d->cursor_.setCursor(cur);
d->cursor_.clearSelection();
+ cur.buffer()->updateStats();
}
d->cursor_.boundary(cur.boundary());
d->cursor_.finishUndo();
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index ce0281251a..49522fed11 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -254,6 +254,7 @@ void CursorData::setSelection()
pit() == normalAnchor().pit() &&
pos() == normalAnchor().pos())
selection(false);
+ buffer()->updateStats();
}
@@ -263,6 +264,7 @@ void CursorData::setSelection(DocIterator const & where, int n)
selection(true);
anchor_ = where;
pos() += n;
+ buffer()->updateStats();
}
@@ -1394,6 +1396,7 @@ bool Cursor::selHandle(bool selecting)
resetAnchor();
selection(selecting);
+ buffer()->updateStats();
return true;
}
@@ -2284,6 +2287,7 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
setCurrentFont();
updateNeeded |= bv().checkDepm(*this, old);
+ buffer()->updateStats();
}
if (updateNeeded)
@@ -2534,6 +2538,7 @@ void Cursor::checkBufferStructure()
// In case the master has no gui associated with it,
// the TocItem is not updated (part of bug 5699).
buffer()->tocBackend().updateItem(*this);
+ buffer()->updateStats();
}
diff --git a/src/Text2.cpp b/src/Text2.cpp
index 3dae072cec..54cd27fc80 100644
--- a/src/Text2.cpp
+++ b/src/Text2.cpp
@@ -552,6 +552,8 @@ bool Text::setCursor(Cursor & cur, pit_type pit, pos_type pos,
bool const update_needed = !tm.contains(pit);
Cursor old = cur;
setCursorIntern(cur, pit, pos, setfont, boundary);
+ if (cur.selection())
+ cur.buffer()->updateStats();
return cur.bv().checkDepm(cur, old) || update_needed;
}
diff --git a/src/Text3.cpp b/src/Text3.cpp
index da23857c1a..86feb5e704 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -2877,6 +2877,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
case LFUN_ESCAPE:
if (cur.selection()) {
cur.selection(false);
+ cur.buffer()->updateStats();
} else {
cur.undispatched();
// This used to be LFUN_FINISHED_RIGHT, I think FORWARD is more
diff --git a/src/frontends/Delegates.h b/src/frontends/Delegates.h
index c50df6b33f..98f672064f 100644
--- a/src/frontends/Delegates.h
+++ b/src/frontends/Delegates.h
@@ -71,6 +71,8 @@ public:
virtual void setBusy(bool) = 0;
/// Reset autosave timers for all users.
virtual void resetAutosaveTimers() = 0;
+ /// Update status bar statistics
+ virtual void updateStats(bool threaded = true) = 0;
};
} // namespace frontend
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index b87b494dac..4f6aa1248b 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -523,7 +523,6 @@ public:
///
QTimer statusbar_timer_;
- QTimer statusbar_stats_timer_;
/// auto-saving of buffers
Timeout autosave_timeout_;
@@ -533,6 +532,7 @@ public:
///
QFutureWatcher autosave_watcher_;
QFutureWatcher processing_thread_watcher_;
+ QFutureWatcher stats_watcher_;
///
string last_export_format;
string processing_format;
@@ -547,20 +547,6 @@ public:
/// flag against a race condition due to multiclicks, see bug #1119
bool in_show_;
-
- // Timers for statistic updates
Re: [patch] Selection stats in statusbar
Indeed, threads create lots of interesting bugs... It would be better if we could avoid them JMarc Le 18 août 2022 18:13:38 GMT+02:00, "Jürgen Spitzmüller" a écrit >Performance is an issue in larger documents (e.g., UserGuide). > >This one, using threads, is snappy and seems to update reliably. > >However, it crashes on each undo or redo action. -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Donnerstag, dem 18.08.2022 um 11:42 +0200 schrieb Jürgen
Spitzmüller:
> The attached seems to work basically. But I haven't check for all
> cases (I am dure there are cases I missed) and didn't push hard for
> performance (currently the timer is commented out; if this is too
> expensive, processes should be managed via QFuture).
Performance is an issue in larger documents (e.g., UserGuide).
This one, using threads, is snappy and seems to update reliably.
However, it crashes on each undo or redo action.
--
Jürgen
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index da630da90a..5770a95b9b 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -4223,6 +4223,13 @@ void Buffer::structureChanged() const
}
+void Buffer::updateStats() const
+{
+ if (d->gui_)
+ d->gui_->updateStats();
+}
+
+
void Buffer::errors(string const & err, bool from_master) const
{
if (d->gui_)
@@ -4944,6 +4951,7 @@ void Buffer::updateBuffer() const
{
updateBuffer(UpdateMaster, InternalUpdate);
d->need_update = false;
+ updateStats();
}
diff --git a/src/Buffer.h b/src/Buffer.h
index ba066f724c..1e4d90fa44 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -660,6 +660,8 @@ public:
void updateTocItem(std::string const &, DocIterator const &) const;
/// This function is called when the buffer structure is changed.
void structureChanged() const;
+ /// This function updates the statistics in the stats bar.
+ void updateStats() const;
/// This function is called when some parsing error shows up.
void errors(std::string const & err, bool from_master = false) const;
/// This function is called when the buffer busy status change.
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index ce0281251a..fd7ac3c484 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -254,6 +254,7 @@ void CursorData::setSelection()
pit() == normalAnchor().pit() &&
pos() == normalAnchor().pos())
selection(false);
+ buffer()->updateStats();
}
@@ -263,6 +264,7 @@ void CursorData::setSelection(DocIterator const & where, int n)
selection(true);
anchor_ = where;
pos() += n;
+ buffer()->updateStats();
}
@@ -501,6 +503,7 @@ void CursorData::clearSelection()
setWordSelection(false);
setMark(false);
resetAnchor();
+ buffer()->updateStats();
}
@@ -1394,6 +1397,7 @@ bool Cursor::selHandle(bool selecting)
resetAnchor();
selection(selecting);
+ buffer()->updateStats();
return true;
}
@@ -2284,6 +2288,7 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
setCurrentFont();
updateNeeded |= bv().checkDepm(*this, old);
+ buffer()->updateStats();
}
if (updateNeeded)
@@ -2534,6 +2539,7 @@ void Cursor::checkBufferStructure()
// In case the master has no gui associated with it,
// the TocItem is not updated (part of bug 5699).
buffer()->tocBackend().updateItem(*this);
+ buffer()->updateStats();
}
diff --git a/src/Text2.cpp b/src/Text2.cpp
index 3dae072cec..54cd27fc80 100644
--- a/src/Text2.cpp
+++ b/src/Text2.cpp
@@ -552,6 +552,8 @@ bool Text::setCursor(Cursor & cur, pit_type pit, pos_type pos,
bool const update_needed = !tm.contains(pit);
Cursor old = cur;
setCursorIntern(cur, pit, pos, setfont, boundary);
+ if (cur.selection())
+ cur.buffer()->updateStats();
return cur.bv().checkDepm(cur, old) || update_needed;
}
diff --git a/src/Text3.cpp b/src/Text3.cpp
index da23857c1a..86feb5e704 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -2877,6 +2877,7 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
case LFUN_ESCAPE:
if (cur.selection()) {
cur.selection(false);
+ cur.buffer()->updateStats();
} else {
cur.undispatched();
// This used to be LFUN_FINISHED_RIGHT, I think FORWARD is more
diff --git a/src/frontends/Delegates.h b/src/frontends/Delegates.h
index c50df6b33f..389a4e47d6 100644
--- a/src/frontends/Delegates.h
+++ b/src/frontends/Delegates.h
@@ -71,6 +71,8 @@ public:
virtual void setBusy(bool) = 0;
/// Reset autosave timers for all users.
virtual void resetAutosaveTimers() = 0;
+ /// Update status bar statistics
+ virtual void updateStats() = 0;
};
} // namespace frontend
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index edcc99983f..5218eef5bb 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -523,7 +523,6 @@ public:
///
QTimer statusbar_timer_;
- QTimer statusbar_stats_timer_;
/// auto-saving of buffers
Timeout autosave_timeout_;
@@ -533,6 +532,7 @@ public:
///
QFutureWatcher autosave_watcher_;
QFutureWatcher processing_thread_watcher_;
+ QFutureWatcher stats_watcher_;
///
string last_export_format;
string processing_format;
@@ -547,20 +547,6 @@ public:
/// flag against a race condition due to multiclicks, see bug #1119
bool in_show_;
-
- // Timers for statistic updates in buffer
- /// Current time left to the nearest info update
- int time_to_update = 1000;
- ///Basic step for timer in ms. Basically reaction time for short selections
- int
Re: [patch] Selection stats in statusbar
Le 18 août 2022 14:25:28 GMT+02:00, "Jürgen Spitzmüller" a écrit : >> My method: statistics update only when idle for a long enough time >> and only once, hence, cannot interfere with continuous typing and are >> very unlikely to interfere at all. > >And it updated after that timing whether it is needed or not. This is >the point I want to bring across. And the delay is not needed if >everything is done properly. It is just a result of bad design. I only read half of this thread, but I the timer could return early if it is determined that the buffer did not change (for example that the undo group number is the same ?). Note also that the stats code can be made much faster with a rewrite without do iterator, if I am not mistaken. But this is for later. JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 18/08/2022 14:25, Jürgen Spitzmüller wrote: Am Donnerstag, dem 18.08.2022 um 14:08 +0200 schrieb Daniel: No function exits early on that approach. The function does not get called in the first place. I think it is really better. Currently: statistics update continuously and, hence, can interfere with continuous typing. Which is not a problem if we use threading. I am wondering whether this is taking a sledgehammer to crack a nut. But since I don't know much about multithreading, I cannot really comment on it. Apparently, it can come with certain risks of indeterminate behaviour and crashes. But again, I don't know whether it applies to this case. As far as I understand it, you will still have to cancel the calculation (as I did stop the timer) if something changes because otherwise, you would come up with a wrong word-count update in case the document has changed in between. My method: statistics update only when idle for a long enough time and only once, hence, cannot interfere with continuous typing and are very unlikely to interfere at all. And it updated after that timing whether it is needed or not. This is the point I want to bring across. And the delay is not needed if everything is done properly. It is just a result of bad design. There is a difference here between the general usage of a timer and the usage of it in showMessage(). It might be that due to the latter (but not the former) it is updated when not needed. However, it does so only once and when idle. If the timer is started instead only for actual changes, there is no such unnecessary update. Though I am still unsure whether it is an issue really due to the idle condition. Having said that, I am happy leaving this for you to try it out. I only wanted to give some comments that came to my mind. Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Donnerstag, dem 18.08.2022 um 14:08 +0200 schrieb Daniel: > No function exits early on that approach. The function does not get > called in the first place. I think it is really better. > > Currently: statistics update continuously and, hence, can interfere > with continuous typing. Which is not a problem if we use threading. > My method: statistics update only when idle for a long enough time > and only once, hence, cannot interfere with continuous typing and are > very unlikely to interfere at all. And it updated after that timing whether it is needed or not. This is the point I want to bring across. And the delay is not needed if everything is done properly. It is just a result of bad design. > I am open to this simple method not being good enough but I haven't > seen any evidence for that yet. I fear I am not able to bring up any evidence that will satisfy you. So let's see what others have to say about it. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 18/08/2022 11:44, Jürgen Spitzmüller wrote: Am Donnerstag, dem 18.08.2022 um 11:31 +0200 schrieb Daniel: This shouldn't be an issue. The idea is that statistics are only generated after some time has passed without interruption. They are not shown immediately. Hence, continuous arrow key movement would not trigger statistics at all. (Same for continuous typing which is an advantage to the current implementation which continuously triggers statistics.) It _is_ an issue if you blindly call functions if not needed, even if they exit early. This is really not better than the current timer-based approach. No function exits early on that approach. The function does not get called in the first place. I think it is really better. Currently: statistics update continuously and, hence, can interfere with continuous typing. My method: statistics update only when idle for a long enough time and only once, hence, cannot interfere with continuous typing and are very unlikely to interfere at all. I am open to this simple method not being good enough but I haven't seen any evidence for that yet. Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Thu, Aug 18, 2022 at 11:44:49AM +0200, Jürgen Spitzmüller wrote: > Am Donnerstag, dem 18.08.2022 um 11:31 +0200 schrieb Daniel: > > This shouldn't be an issue. The idea is that statistics are only > > generated after some time has passed without interruption. They are > > not > > shown immediately. Hence, continuous arrow key movement would not > > trigger statistics at all. (Same for continuous typing which is an > > advantage to the current implementation which continuously triggers > > statistics.) > > It _is_ an issue if you blindly call functions if not needed, even if > they exit early. This is really not better than the current timer-based > approach. > > Statistics should only be called when needed. I agree. Even if the hit doesn't show up in noticeable lag, or even in profiling, it does not seem like clean coding and I don't have any intuition for what could change down the road (e.g., maybe we increase the use of showMessage() by an order of magnitude for some other purpose). Maybe for a bug fix, things like this can be acceptable, but for a feature we try to keep the bar high (although of course there are exceptions). Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Donnerstag, dem 18.08.2022 um 11:31 +0200 schrieb Daniel: > This shouldn't be an issue. The idea is that statistics are only > generated after some time has passed without interruption. They are > not > shown immediately. Hence, continuous arrow key movement would not > trigger statistics at all. (Same for continuous typing which is an > advantage to the current implementation which continuously triggers > statistics.) It _is_ an issue if you blindly call functions if not needed, even if they exit early. This is really not better than the current timer-based approach. Statistics should only be called when needed. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Donnerstag, dem 18.08.2022 um 09:55 +0200 schrieb Jürgen
Spitzmüller:
> You need a more suitable signal that only emits when text is edited
> or loaded.
The attached seems to work basically. But I haven't check for all cases
(I am dure there are cases I missed) and didn't push hard for
performance (currently the timer is commented out; if this is too
expensive, processes should be managed via QFuture).
--
Jürgen
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index da630da90a..0db97bea16 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -4223,6 +4223,13 @@ void Buffer::structureChanged() const
}
+void Buffer::updateStats() const
+{
+ if (d->gui_)
+ d->gui_->showStats();
+}
+
+
void Buffer::errors(string const & err, bool from_master) const
{
if (d->gui_)
@@ -4944,6 +4951,7 @@ void Buffer::updateBuffer() const
{
updateBuffer(UpdateMaster, InternalUpdate);
d->need_update = false;
+ updateStats();
}
diff --git a/src/Buffer.h b/src/Buffer.h
index ba066f724c..1e4d90fa44 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -660,6 +660,8 @@ public:
void updateTocItem(std::string const &, DocIterator const &) const;
/// This function is called when the buffer structure is changed.
void structureChanged() const;
+ /// This function updates the statistics in the stats bar.
+ void updateStats() const;
/// This function is called when some parsing error shows up.
void errors(std::string const & err, bool from_master = false) const;
/// This function is called when the buffer busy status change.
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index ce0281251a..fd7ac3c484 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -254,6 +254,7 @@ void CursorData::setSelection()
pit() == normalAnchor().pit() &&
pos() == normalAnchor().pos())
selection(false);
+ buffer()->updateStats();
}
@@ -263,6 +264,7 @@ void CursorData::setSelection(DocIterator const & where, int n)
selection(true);
anchor_ = where;
pos() += n;
+ buffer()->updateStats();
}
@@ -501,6 +503,7 @@ void CursorData::clearSelection()
setWordSelection(false);
setMark(false);
resetAnchor();
+ buffer()->updateStats();
}
@@ -1394,6 +1397,7 @@ bool Cursor::selHandle(bool selecting)
resetAnchor();
selection(selecting);
+ buffer()->updateStats();
return true;
}
@@ -2284,6 +2288,7 @@ bool Cursor::upDownInText(bool up, bool & updateNeeded)
setCurrentFont();
updateNeeded |= bv().checkDepm(*this, old);
+ buffer()->updateStats();
}
if (updateNeeded)
@@ -2534,6 +2539,7 @@ void Cursor::checkBufferStructure()
// In case the master has no gui associated with it,
// the TocItem is not updated (part of bug 5699).
buffer()->tocBackend().updateItem(*this);
+ buffer()->updateStats();
}
diff --git a/src/Text2.cpp b/src/Text2.cpp
index 3dae072cec..54cd27fc80 100644
--- a/src/Text2.cpp
+++ b/src/Text2.cpp
@@ -552,6 +552,8 @@ bool Text::setCursor(Cursor & cur, pit_type pit, pos_type pos,
bool const update_needed = !tm.contains(pit);
Cursor old = cur;
setCursorIntern(cur, pit, pos, setfont, boundary);
+ if (cur.selection())
+ cur.buffer()->updateStats();
return cur.bv().checkDepm(cur, old) || update_needed;
}
diff --git a/src/frontends/Delegates.h b/src/frontends/Delegates.h
index c50df6b33f..7545581c4c 100644
--- a/src/frontends/Delegates.h
+++ b/src/frontends/Delegates.h
@@ -71,6 +71,8 @@ public:
virtual void setBusy(bool) = 0;
/// Reset autosave timers for all users.
virtual void resetAutosaveTimers() = 0;
+ /// Update status bar statistics
+ virtual void showStats() = 0;
};
} // namespace frontend
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index edcc99983f..251781951a 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -523,7 +523,6 @@ public:
///
QTimer statusbar_timer_;
- QTimer statusbar_stats_timer_;
/// auto-saving of buffers
Timeout autosave_timeout_;
@@ -598,9 +597,6 @@ GuiView::GuiView(int id)
}
connect(_timer_, SIGNAL(timeout()),
this, SLOT(clearMessage()));
- connect(_stats_timer_, SIGNAL(timeout()),
- this, SLOT(showStats()));
- d.statusbar_stats_timer_.start(d.timer_rate);
// We don't want to keep the window in memory if it is closed.
setAttribute(Qt::WA_DeleteOnClose, true);
@@ -1312,7 +1308,6 @@ void GuiView::closeEvent(QCloseEvent * close_event)
// Make sure the timer time out will not trigger a statusbar update.
d.statusbar_timer_.stop();
- d.statusbar_stats_timer_.stop();
// Saving fullscreen requires additional tweaks in the toolbar code.
// It wouldn't also work under linux natively.
@@ -1425,7 +1420,7 @@ void GuiView::showStats()
d.time_to_update -= d.timer_rate;
- BufferView * bv = currentBufferView();
+ BufferView * bv = documentBufferView();
Buffer * buf = bv ? >buffer() : nullptr;
if (!buf) {
stat_counts_->hide();
@@ -1438,8 +1433,8 @@ void GuiView::showStats()
if (!d.already_in_selection_ && cur.selection())
Re: [patch] Selection stats in statusbar
On 18/08/2022 09:55, Jürgen Spitzmüller wrote: Am Mittwoch, dem 17.08.2022 um 19:40 +0200 schrieb Daniel: What I did in https://www.lyx.org/trac/attachment/ticket/12422/0001-Show-word-count-in-status-bar.patch was just to hook it on GuiView::showMessage() because this would be updated in all cases where text stuff changes. showMessage() is triggered on any cursor action. That's way too often. E.g., it would fire down statistics unnecessarily if you move with the arrow keys through your document. This shouldn't be an issue. The idea is that statistics are only generated after some time has passed without interruption. They are not shown immediately. Hence, continuous arrow key movement would not trigger statistics at all. (Same for continuous typing which is an advantage to the current implementation which continuously triggers statistics.) And I am not completely sure each relevant action triggers a message. You need a more suitable signal that only emits when text is edited or loaded. I don't know what the "internal processes" are though. You can use lfuns e.g. via the LyXServer mechanism or pipes. This can also be used to insert text or citations. This seems to trigger showMessage() as well. Unless, we find a case where showMessage() wouldn't be triggered, it still seems to me worth a try before digging deeper. Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 17.08.2022 um 19:40 +0200 schrieb Daniel: > What I did in > > https://www.lyx.org/trac/attachment/ticket/12422/0001-Show-word-count-in-status-bar.patch > > was just to hook it on GuiView::showMessage() because this would be > updated in all cases where text stuff changes. showMessage() is triggered on any cursor action. That's way too often. E.g., it would fire down statistics unnecessarily if you move with the arrow keys through your document. And I am not completely sure each relevant action triggers a message. You need a more suitable signal that only emits when text is edited or loaded. > I don't know what the "internal processes" are though. You can use lfuns e.g. via the LyXServer mechanism or pipes. This can also be used to insert text or citations. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 2022-08-17 18:41, Jürgen Spitzmüller wrote: Am Mittwoch, dem 10.08.2022 um 08:01 +0200 schrieb Daniel: 1. The delay timer seems not to be working as expected. Normally, i.e. in other word processors, the idea is this: While typing, no updating takes place, only once the typing stops, the update timer starts. Once typing is continued, the update timer is stopped. I guess this is to make sure that calculation of the statistics is very unlikely to interfere with the typing. However, as it is now, the word count gets updated sometimes while continuously typing. I thought that I had figured this out in my patch but maybe not. (Or that is a part where you where wondering why it was as it was.) I agree this would be much better. Also, async calculation (via QFuture) would probably help with managing the calculation processes and mitigate delays. What is not so easy, I think, is to trigger correctly when the statistics should be updated. This is not only on typing, but also on pasting, cutting, undo/redo, buffer loading and buffer switching. Maybe even on internal processes (think LyXServer). I onl y had a quick look, but I think the signals would have to be emitted from many different places. What I did in https://www.lyx.org/trac/attachment/ticket/12422/0001-Show-word-count-in-status-bar.patch was just to hook it on GuiView::showMessage() because this would be updated in all cases where text stuff changes. It seemed to work. I don't know what the "internal processes" are though. Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 10.08.2022 um 08:01 +0200 schrieb Daniel: > 1. The delay timer seems not to be working as expected. Normally, > i.e. > in other word processors, the idea is this: > > While typing, no updating takes place, only once the typing stops, > the > update timer starts. Once typing is continued, the update timer is > stopped. I guess this is to make sure that calculation of the > statistics > is very unlikely to interfere with the typing. > > However, as it is now, the word count gets updated sometimes while > continuously typing. I thought that I had figured this out in my > patch > but maybe not. (Or that is a part where you where wondering why it > was > as it was.) I agree this would be much better. Also, async calculation (via QFuture) would probably help with managing the calculation processes and mitigate delays. What is not so easy, I think, is to trigger correctly when the statistics should be updated. This is not only on typing, but also on pasting, cutting, undo/redo, buffer loading and buffer switching. Maybe even on internal processes (think LyXServer). I onl y had a quick look, but I think the signals would have to be emitted from many different places. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 11/08/2022 08:54, Pavel Sanda wrote: On Wed, Aug 10, 2022 at 08:01:47AM +0200, Daniel wrote: I just tested the new feature. Here are a couple of things that came to my mind: 1. The delay timer seems not to be working as expected. Normally, i.e. in other word processors, the idea is this: While typing, no updating takes place, only once the typing stops, the update timer starts. Once typing is continued, the update timer is stopped. I guess this is to make sure that calculation of the statistics is very unlikely to interfere with the typing. This seems reasonable approach, actually better then the current one. However, as it is now, the word count gets updated sometimes while continuously typing. I thought that I had figured this out in my patch but maybe not. (Or that is a part where you where wondering why it was as it was.) Yes, your patch didn't make it clear in the comments how it works, and it was not straightforward from the code what is the combination if stops/starts supposed to do. This makes it hard to anyone from outside to fix if something breaks. If you are willing to improve it I am happy to push it into master. I will re-check your patch now that it is in master and see whether I can add the feature. Yes, I agree that I did not comment my patch well. I wasn't sure whether there would be interest, so it was only intended for people to try it out. (I also wasn't sure from your earlier feedback whether you would have liked more comments on my patch after I asked whether I should add more comments.) Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 17.08.2022 um 14:21 +0200 schrieb Pavel Sanda: > I am travelling now, so please do as you see fit, I trust your UI > eye. Thanks, done. Have a good trip. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Wed, Aug 17, 2022 at 11:00:39AM +0200, Jürgen Spitzmüller wrote: > Am Dienstag, dem 16.08.2022 um 12:44 +0200 schrieb Jürgen Spitzmüller: > > I've seen now that you flagged it for localization. But this is still > > to anglocentric. Use bformat to construct the string. BTW there is > > also > > a qt_() function, no need for toqstr(_()). > > > > My point that one-letter abbreviations in UI are a no-go still holds. > > Attached the way I'd recommend to do it: I am travelling now, so please do as you see fit, I trust your UI eye. Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Wed, 17 Aug 2022 12:39:01 +0200 schrieb "Jürgen Spitzmüller" : > Am Mittwoch, dem 17.08.2022 um 12:35 +0200 schrieb Daniel: > > 1 slovo > > Počet slov: 2... (Wordcount: 2... if I understood correctly) > > This could be done on po-level if desired. No code change needed. It is that way in my local sk.po. Kornel pgpMv5gdO8HDN.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Wed, 17 Aug 2022 12:35:05 +0200 schrieb Daniel : > On 17/08/2022 12:01, Jürgen Spitzmüller wrote: > > Am Mittwoch, dem 17.08.2022 um 11:56 +0200 schrieb Jürgen Spitzmüller: > >> If the number of inter-language variations is limited, this can be > >> supported as well. > > > > OTOH the problem is also unsolved in the existing statistics dialog. > > Word uses > > 1 slovo > Počet slov: 2... (Wordcount: 2... if I understood correctly) > > Daniel > Yes, this would be independent of the number. The correct back-translation reads: Number of words: 2 Kornel pgpMNfzt67eoM.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 17.08.2022 um 12:35 +0200 schrieb Daniel: > 1 slovo > Počet slov: 2... (Wordcount: 2... if I understood correctly) This could be done on po-level if desired. No code change needed. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 17/08/2022 12:01, Jürgen Spitzmüller wrote: Am Mittwoch, dem 17.08.2022 um 11:56 +0200 schrieb Jürgen Spitzmüller: If the number of inter-language variations is limited, this can be supported as well. OTOH the problem is also unsolved in the existing statistics dialog. Word uses 1 slovo Počet slov: 2... (Wordcount: 2... if I understood correctly) Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 17/08/2022 11:57, Jürgen Spitzmüller wrote: Am Mittwoch, dem 17.08.2022 um 11:28 +0200 schrieb Daniel: If the usage of QStringList is fine, it can also be written a bit more concise as. Sure. Though it doesn't buy much. I guess one should choose (even slightly) less complex code whenever (easily) possible. Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 17.08.2022 um 11:56 +0200 schrieb Jürgen Spitzmüller: > If the number of inter-language variations is limited, this can be > supported as well. OTOH the problem is also unsolved in the existing statistics dialog. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 17.08.2022 um 11:28 +0200 schrieb Daniel: > If the usage of QStringList is fine, it can also be written a bit > more concise as. Sure. Though it doesn't buy much. > Also, chars_with_blanks should probably be inside the > second "if statement" No, this is re-used further below. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 17.08.2022 um 11:41 +0200 schrieb Kornel Benko: > What about languages with different plurals. > > In Slovak for example it would be > 1 slovo > 2..4 slová > 5..\infty slov If the number of inter-language variations is limited, this can be supported as well. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Wed, 17 Aug 2022 11:28:53 +0200
schrieb Daniel :
> On 17/08/2022 11:13, Jürgen Spitzmüller wrote:
> > Am Mittwoch, dem 17.08.2022 um 11:00 +0200 schrieb Jürgen Spitzmüller:
> >> Attached the way I'd recommend to do it:
> >>
> >> * do not use abbreviations
> >> * allow to enable the stats measures individually
> >> * left click on stats in status bar opens stats dialog (as in LO and
> >> Word)
> >>
> >> The rationale besides the accessibility argument is that I'd figure
> >> that users seldom need all three stats values. The value they need
> >> might depend on the editorial instructions they get. These might rely
> >> on either of the three. So give the user the choice to see what they
> >> need.
> >
> > Updated patch which accounts for the proper number (1 Word, 2 Words).
>
> If the usage of QStringList is fine, it can also be written a bit more
> concise as. Also, chars_with_blanks should probably be inside the second
> "if statement":
>
> QStringList stats;
> if (word_count_enabled_) {
> int const words = buf->wordCount();
> if (words == 1)
> stats << toqstr(bformat(_("%1$d Word"), words));
> else
> stats << toqstr(bformat(_("%1$d Words"), words));
> }
> if (char_count_enabled_)
> int const chars_with_blanks = buf->charCount(true);
> if (chars_with_blanks == 1)
> stats << toqstr(bformat(_("%1$d Character"),
> chars_with_blanks));
> else
> stats << toqstr(bformat(_("%1$d Characters"),
> chars_with_blanks));
> if (char_nb_count_enabled_) {
> int const chars = buf->charCount(false);
> if (chars == 1)
> stats << toqstr(bformat(_("%1$d Character (no Blanks)"),
> chars));
> else
> stats << toqstr(bformat(_("%1$d Characters (no Blanks)"),
> chars));
> }
> stat_counts_->setText(stats.join(qt_(", [[stats separator]]")));
>
>
What about languages with different plurals.
In Slovak for example it would be
1 slovo
2..4 slová
5..\infty slov
Kornel
pgpauZzbP2jRR.pgp
Description: Digitale Signatur von OpenPGP
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 17/08/2022 11:13, Jürgen Spitzmüller wrote:
Am Mittwoch, dem 17.08.2022 um 11:00 +0200 schrieb Jürgen Spitzmüller:
Attached the way I'd recommend to do it:
* do not use abbreviations
* allow to enable the stats measures individually
* left click on stats in status bar opens stats dialog (as in LO and
Word)
The rationale besides the accessibility argument is that I'd figure
that users seldom need all three stats values. The value they need
might depend on the editorial instructions they get. These might rely
on either of the three. So give the user the choice to see what they
need.
Updated patch which accounts for the proper number (1 Word, 2 Words).
If the usage of QStringList is fine, it can also be written a bit more
concise as. Also, chars_with_blanks should probably be inside the second
"if statement":
QStringList stats;
if (word_count_enabled_) {
int const words = buf->wordCount();
if (words == 1)
stats << toqstr(bformat(_("%1$d Word"), words));
else
stats << toqstr(bformat(_("%1$d Words"), words));
}
if (char_count_enabled_)
int const chars_with_blanks = buf->charCount(true);
if (chars_with_blanks == 1)
stats << toqstr(bformat(_("%1$d Character"),
chars_with_blanks));
else
stats << toqstr(bformat(_("%1$d Characters"),
chars_with_blanks));
if (char_nb_count_enabled_) {
int const chars = buf->charCount(false);
if (chars == 1)
stats << toqstr(bformat(_("%1$d Character (no Blanks)"),
chars));
else
stats << toqstr(bformat(_("%1$d Characters (no Blanks)"),
chars));
}
stat_counts_->setText(stats.join(qt_(", [[stats separator]]")));
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Mittwoch, dem 17.08.2022 um 11:00 +0200 schrieb Jürgen Spitzmüller:
> Attached the way I'd recommend to do it:
>
> * do not use abbreviations
> * allow to enable the stats measures individually
> * left click on stats in status bar opens stats dialog (as in LO and
> Word)
>
> The rationale besides the accessibility argument is that I'd figure
> that users seldom need all three stats values. The value they need
> might depend on the editorial instructions they get. These might rely
> on either of the three. So give the user the choice to see what they
> need.
Updated patch which accounts for the proper number (1 Word, 2 Words).
--
Jürgen
diff --git a/lib/ui/stdcontext.inc b/lib/ui/stdcontext.inc
index 554798c11a..8eb8ddde82 100644
--- a/lib/ui/stdcontext.inc
+++ b/lib/ui/stdcontext.inc
@@ -732,7 +732,9 @@ Menuset
Item "Show Zoom Level|Z" "ui-toggle zoomlevel"
Item "Show Zoom Slider|S" "ui-toggle zoomslider"
Separator
- Item "Show Statistics|t" "ui-toggle statistics"
+ Item "Show Word Count|W" "ui-toggle statistics-w"
+ Item "Show Character Count (Incl. Blanks)|C" "ui-toggle statistics-cb"
+ Item "Show Character Count (Without Blanks)|h" "ui-toggle statistics-c"
End
End
diff --git a/src/LyXAction.cpp b/src/LyXAction.cpp
index ae8245d070..6c80fb1d37 100644
--- a/src/LyXAction.cpp
+++ b/src/LyXAction.cpp
@@ -4091,16 +4091,20 @@ void LyXAction::init()
* \var lyx::FuncCode lyx::LFUN_UI_TOGGLE
* \li Action: Various UI visibility-toggling actions.
* \li Syntax: ui-toggle
- * \li Params: statusbar : Toggle visibility of the statusbar.\n
- menubar: Toggle visibility of the menubar.\n
- scrollbar : Toggle visibility of the scrollbar.\n
- frame : Toggle visibility of the frames around editing window.\n
- zoomslider : Toggle visibility of the zoom slider in statusbar.\n
- zoomlevel : Toggle visibility of the zoom level display in statusbar.\n
- statistics : Toggle visibility of the document statistics count in statusbar.\n
- fullscreen : Toggle fullscreen mode. This also covers calling the
-previous functions. However #LFUN_TOOLBAR_TOGGLE for the
-custom tweaks of the toolbars should be used.
+ * \li Params: statusbar : Toggle visibility of the statusbar.\n
+ menubar : Toggle visibility of the menubar.\n
+ scrollbar : Toggle visibility of the scrollbar.\n
+ frame : Toggle visibility of the frames around editing window.\n
+ zoomslider: Toggle visibility of the zoom slider in statusbar.\n
+ zoomlevel : Toggle visibility of the zoom level display in statusbar.\n
+ statistics-w : Toggle visibility of the document word count in statusbar.\n
+ statistics-cb : Toggle visibility of the document character count (incl. blanks)
+ in statusbar.\n
+ statistics-c : Toggle visibility of the document character count (excl. blanks)
+ in statusbar.\n
+ fullscreen: Toggle fullscreen mode. This also covers calling the
+ previous functions. However #LFUN_TOOLBAR_TOGGLE for the
+ custom tweaks of the toolbars should be used.
* \li Origin: sanda, 9 Feb 2007
* \endvar
*/
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index d03fe3bc2f..89efc62176 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -568,7 +568,8 @@ QSet GuiView::GuiViewPrivate::busyBuffers;
GuiView::GuiView(int id)
: d(*new GuiViewPrivate(this)), id_(id), closing_(false), busy_(0),
- command_execute_(false), minibuffer_focus_(false), stat_counts_enabled_(true),
+ command_execute_(false), minibuffer_focus_(false), word_count_enabled_(true),
+ char_count_enabled_(true), char_nb_count_enabled_(false),
toolbarsMovable_(true), devel_mode_(false)
{
connect(this, SIGNAL(bufferViewChanged()),
@@ -645,12 +646,14 @@ GuiView::GuiView(int id)
busySVG, SLOT(hide()));
connect(busySVG, SIGNAL(pressed()), this, SLOT(checkCancelBackground()));
- stat_counts_ = new QLabel(statusBar());
+ stat_counts_ = new GuiClickableLabel(statusBar());
stat_counts_->setAlignment(Qt::AlignCenter);
stat_counts_->setFrameStyle(QFrame::StyledPanel);
stat_counts_->hide();
statusBar()->addPermanentWidget(stat_counts_);
+ connect(stat_counts_, SIGNAL(clicked()), this, SLOT(statsPressed()));
+
QFontMetrics const fm(statusBar()->fontMetrics());
@@ -820,6 +823,11 @@ void GuiView::checkCancelBackground()
Systemcall::killscript();
}
+void GuiView::statsPressed()
+{
+ DispatchResult dr;
+ dispatch(FuncRequest(LFUN_STATISTICS), dr);
+}
void GuiView::zoomSliderMoved(int value)
{
@@ -977,7 +985,9 @@ void GuiView::saveLayout() const
Re: [patch] Selection stats in statusbar
Am Dienstag, dem 16.08.2022 um 12:44 +0200 schrieb Jürgen Spitzmüller:
> I've seen now that you flagged it for localization. But this is still
> to anglocentric. Use bformat to construct the string. BTW there is
> also
> a qt_() function, no need for toqstr(_()).
>
> My point that one-letter abbreviations in UI are a no-go still holds.
Attached the way I'd recommend to do it:
* do not use abbreviations
* allow to enable the stats measures individually
* left click on stats in status bar opens stats dialog (as in LO and
Word)
The rationale besides the accessibility argument is that I'd figure
that users seldom need all three stats values. The value they need
might depend on the editorial instructions they get. These might rely
on either of the three. So give the user the choice to see what they
need.
--
Jürgen
diff --git a/lib/ui/stdcontext.inc b/lib/ui/stdcontext.inc
index 554798c11a..8eb8ddde82 100644
--- a/lib/ui/stdcontext.inc
+++ b/lib/ui/stdcontext.inc
@@ -732,7 +732,9 @@ Menuset
Item "Show Zoom Level|Z" "ui-toggle zoomlevel"
Item "Show Zoom Slider|S" "ui-toggle zoomslider"
Separator
- Item "Show Statistics|t" "ui-toggle statistics"
+ Item "Show Word Count|W" "ui-toggle statistics-w"
+ Item "Show Character Count (Incl. Blanks)|C" "ui-toggle statistics-cb"
+ Item "Show Character Count (Without Blanks)|h" "ui-toggle statistics-c"
End
End
diff --git a/src/LyXAction.cpp b/src/LyXAction.cpp
index ae8245d070..6c80fb1d37 100644
--- a/src/LyXAction.cpp
+++ b/src/LyXAction.cpp
@@ -4091,16 +4091,20 @@ void LyXAction::init()
* \var lyx::FuncCode lyx::LFUN_UI_TOGGLE
* \li Action: Various UI visibility-toggling actions.
* \li Syntax: ui-toggle
- * \li Params: statusbar : Toggle visibility of the statusbar.\n
- menubar: Toggle visibility of the menubar.\n
- scrollbar : Toggle visibility of the scrollbar.\n
- frame : Toggle visibility of the frames around editing window.\n
- zoomslider : Toggle visibility of the zoom slider in statusbar.\n
- zoomlevel : Toggle visibility of the zoom level display in statusbar.\n
- statistics : Toggle visibility of the document statistics count in statusbar.\n
- fullscreen : Toggle fullscreen mode. This also covers calling the
-previous functions. However #LFUN_TOOLBAR_TOGGLE for the
-custom tweaks of the toolbars should be used.
+ * \li Params: statusbar : Toggle visibility of the statusbar.\n
+ menubar : Toggle visibility of the menubar.\n
+ scrollbar : Toggle visibility of the scrollbar.\n
+ frame : Toggle visibility of the frames around editing window.\n
+ zoomslider: Toggle visibility of the zoom slider in statusbar.\n
+ zoomlevel : Toggle visibility of the zoom level display in statusbar.\n
+ statistics-w : Toggle visibility of the document word count in statusbar.\n
+ statistics-cb : Toggle visibility of the document character count (incl. blanks)
+ in statusbar.\n
+ statistics-c : Toggle visibility of the document character count (excl. blanks)
+ in statusbar.\n
+ fullscreen: Toggle fullscreen mode. This also covers calling the
+ previous functions. However #LFUN_TOOLBAR_TOGGLE for the
+ custom tweaks of the toolbars should be used.
* \li Origin: sanda, 9 Feb 2007
* \endvar
*/
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index d03fe3bc2f..043a839caa 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -568,7 +568,8 @@ QSet GuiView::GuiViewPrivate::busyBuffers;
GuiView::GuiView(int id)
: d(*new GuiViewPrivate(this)), id_(id), closing_(false), busy_(0),
- command_execute_(false), minibuffer_focus_(false), stat_counts_enabled_(true),
+ command_execute_(false), minibuffer_focus_(false), word_count_enabled_(true),
+ char_count_enabled_(true), char_nb_count_enabled_(false),
toolbarsMovable_(true), devel_mode_(false)
{
connect(this, SIGNAL(bufferViewChanged()),
@@ -645,12 +646,14 @@ GuiView::GuiView(int id)
busySVG, SLOT(hide()));
connect(busySVG, SIGNAL(pressed()), this, SLOT(checkCancelBackground()));
- stat_counts_ = new QLabel(statusBar());
+ stat_counts_ = new GuiClickableLabel(statusBar());
stat_counts_->setAlignment(Qt::AlignCenter);
stat_counts_->setFrameStyle(QFrame::StyledPanel);
stat_counts_->hide();
statusBar()->addPermanentWidget(stat_counts_);
+ connect(stat_counts_, SIGNAL(clicked()), this, SLOT(statsPressed()));
+
QFontMetrics const fm(statusBar()->fontMetrics());
@@ -820,6 +823,11 @@ void GuiView::checkCancelBackground()
Systemcall::killscript();
}
+void GuiView::statsPressed()
+{
+
Re: [patch] Selection stats in statusbar
Am Dienstag, dem 16.08.2022 um 12:12 +0200 schrieb Jürgen Spitzmüller: > It's horrible, really. You need not only to spell it out, but also > localize it. I've seen now that you flagged it for localization. But this is still to anglocentric. Use bformat to construct the string. BTW there is also a qt_() function, no need for toqstr(_()). My point that one-letter abbreviations in UI are a no-go still holds. -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Am Donnerstag, dem 11.08.2022 um 08:54 +0200 schrieb Pavel Sanda: > > 2. The display is quite cryptic since it is not immediately clear > > what "w", > > "c" and "cb" is. Minimally, I suggest to add a tooltip explaining > > this. > > Ideally, I would write at least "3 words, 23 characters" out, as it > > is the > > case in other word processors (Writer and Word). > > > > I see that this might get a bit longish if "characters including > > blanks" is > > written out. Here are a couple of alternatives: > > > > a. abbreviate only these: "42 cb". > > b. show blanks: "19 blanks". > > I agree with initital cryptic impression and was expectecting someone > will question it. For me, once you get what w/c/cb is supposed to > mean the current display is easier to parse, especially if you are > actively changing the selection size and need quick feedback while > selecting. > We can change it if others will complain as well. It's horrible, really. You need not only to spell it out, but also localize it. How should a non-English user know what "c" (not to mention "cb") is supposed to mean? If space is an issue, I'd suggest to only list "N words" and "N characters" (where the latter means "incl. blanks", as in the LibreOffice status bar). Also remember while localizing that the word order differs in localizations. BTW does the counting use async processes? -- Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Wed, Aug 10, 2022 at 08:01:47AM +0200, Daniel wrote: > I just tested the new feature. Here are a couple of things that came to my > mind: > > 1. The delay timer seems not to be working as expected. Normally, i.e. in > other word processors, the idea is this: > > While typing, no updating takes place, only once the typing stops, the > update timer starts. Once typing is continued, the update timer is stopped. > I guess this is to make sure that calculation of the statistics is very > unlikely to interfere with the typing. This seems reasonable approach, actually better then the current one. > However, as it is now, the word count gets updated sometimes while > continuously typing. I thought that I had figured this out in my patch but > maybe not. (Or that is a part where you where wondering why it was as it > was.) Yes, your patch didn't make it clear in the comments how it works, and it was not straightforward from the code what is the combination if stops/starts supposed to do. This makes it hard to anyone from outside to fix if something breaks. If you are willing to improve it I am happy to push it into master. > 2. The display is quite cryptic since it is not immediately clear what "w", > "c" and "cb" is. Minimally, I suggest to add a tooltip explaining this. > Ideally, I would write at least "3 words, 23 characters" out, as it is the > case in other word processors (Writer and Word). > > I see that this might get a bit longish if "characters including blanks" is > written out. Here are a couple of alternatives: > > a. abbreviate only these: "42 cb". > b. show blanks: "19 blanks". I agree with initital cryptic impression and was expectecting someone will question it. For me, once you get what w/c/cb is supposed to mean the current display is easier to parse, especially if you are actively changing the selection size and need quick feedback while selecting. We can change it if others will complain as well. I totally agree with tooltip part though. > 3. One should be able to choose ones statics that are shown. I have never > come across a scenario where I had to keep track of both words and > characters. And most of the time it is the same, only words for me. So, it > would be nice, if there was a specific context menu for the statistics that > lets one choose which one to show in order to avoid clutter with extra > information. This doesn't rule out showing multiple statistics at least in > principle and abbreviations could be used depending on the number of > statistics shown. Looks overengineered to me and it actually shows why w/c/cb labels are good - it doesn't feel bloated compared to situation when you need to parse full sentence and start thinking about "new options". I am encountering silly forms requiring characters limits regularly, so choosing the one right stat is not solution either. > 4. At least on macOS, there is currently a border around the statistics in > the status bar. This does not integrate well with the other information > shown which does not have a border. The border-less style seems more clean > to me. I just followed the way we display version control into toolbar (shows border which makes it visually distinct as a separate functionality). But I don't have strong opinion, be my guest if you feel strongly about this. Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 2022-08-09 23:53, Pavel Sanda wrote: On Sat, Aug 06, 2022 at 12:23:25PM -0400, Scott Kostyshak wrote: 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. It's now in 5b50a97fd76d2. Please give it some testing once you back... Thanks, Pavel I just tested the new feature. Here are a couple of things that came to my mind: 1. The delay timer seems not to be working as expected. Normally, i.e. in other word processors, the idea is this: While typing, no updating takes place, only once the typing stops, the update timer starts. Once typing is continued, the update timer is stopped. I guess this is to make sure that calculation of the statistics is very unlikely to interfere with the typing. However, as it is now, the word count gets updated sometimes while continuously typing. I thought that I had figured this out in my patch but maybe not. (Or that is a part where you where wondering why it was as it was.) 2. The display is quite cryptic since it is not immediately clear what "w", "c" and "cb" is. Minimally, I suggest to add a tooltip explaining this. Ideally, I would write at least "3 words, 23 characters" out, as it is the case in other word processors (Writer and Word). I see that this might get a bit longish if "characters including blanks" is written out. Here are a couple of alternatives: a. abbreviate only these: "42 cb". b. show blanks: "19 blanks". 3. One should be able to choose ones statics that are shown. I have never come across a scenario where I had to keep track of both words and characters. And most of the time it is the same, only words for me. So, it would be nice, if there was a specific context menu for the statistics that lets one choose which one to show in order to avoid clutter with extra information. This doesn't rule out showing multiple statistics at least in principle and abbreviations could be used depending on the number of statistics shown. 4. At least on macOS, there is currently a border around the statistics in the status bar. This does not integrate well with the other information shown which does not have a border. The border-less style seems more clean to me. I know that you are busy, so just let me know if I should look into any of these. Thanks, Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Tue, Aug 09, 2022 at 11:53:05PM +0200, Pavel Sanda wrote: > On Sat, Aug 06, 2022 at 12:23:25PM -0400, Scott Kostyshak wrote: > > > 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. > > It's now in 5b50a97fd76d2. > > Please give it some testing once you back... Sounds good, thanks! Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Sat, Aug 06, 2022 at 12:23:25PM -0400, Scott Kostyshak wrote: > > 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. It's now in 5b50a97fd76d2. Please give it some testing once you back... Thanks, Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
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
Re: [patch] Selection stats in statusbar
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 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.
Pavel
diff --git a/lib/ui/stdcontext.inc b/lib/ui/stdcontext.inc
index bd94e7c904..8c79c5f351 100644
--- a/lib/ui/stdcontext.inc
+++ b/lib/ui/stdcontext.inc
@@ -731,6 +731,8 @@ Menuset
Separator
Item "Show Zoom Level|Z" "ui-toggle zoomlevel"
Item "Show Zoom Slider|S" "ui-toggle zoomslider"
+ Separator
+ Item "Show Document Statistics|D" "ui-toggle statistics"
End
End
diff --git a/src/LyXAction.cpp b/src/LyXAction.cpp
index 992b47a441..ae8245d070 100644
--- a/src/LyXAction.cpp
+++ b/src/LyXAction.cpp
@@ -4097,6 +4097,7 @@ void LyXAction::init()
frame : Toggle visibility of the frames around editing
window.\n
zoomslider : Toggle visibility of the zoom slider in
statusbar.\n
zoomlevel : Toggle visibility of the zoom level display in
statusbar.\n
+ statistics : Toggle visibility of the document statistics count
in statusbar.\n
fullscreen : Toggle fullscreen mode. This also covers calling
the
previous functions. However #LFUN_TOOLBAR_TOGGLE
for the
custom tweaks of the toolbars should be used.
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index 8f6bf07115..fd738e100e 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -523,6 +523,7 @@ public:
///
QTimer statusbar_timer_;
+ QTimer statusbar_stats_timer_;
/// auto-saving of buffers
Timeout autosave_timeout_;
@@ -546,6 +547,18 @@ public:
/// flag against a race condition due to multiclicks, see bug #1119
bool in_show_;
+
+ // Timers for statistic updates in buffer
+ /// Current time left to the nearest info update
+ int time_to_update = 1000;
+ ///Basic step for timer in ms. Basically reaction time for short
selections
+ int const timer_rate = 500;
+ /// Real stats updates infrequently. First they take long time for big
buffers, second
+ /// they are visible for fast-repeat keyboards even for mid documents.
+ int const default_stats_rate = 5000;
+ /// Detection of new selection, so we can react fast
+ bool already_in_selection_ = false;
+
};
QSet GuiView::GuiViewPrivate::busyBuffers;
@@ -553,8 +566,8 @@ QSet GuiView::GuiViewPrivate::busyBuffers;
GuiView::GuiView(int id)
: d(*new GuiViewPrivate(this)), id_(id), closing_(false), busy_(0),
- command_execute_(false), minibuffer_focus_(false),
toolbarsMovable_(true),
- devel_mode_(false)
+ command_execute_(false), minibuffer_focus_(false),
stat_counts_enabled_(true),
+ toolbarsMovable_(true), devel_mode_(false)
{
connect(this, SIGNAL(bufferViewChanged()),
this, SLOT(onBufferViewChanged()));
@@ -582,6 +595,9 @@ GuiView::GuiView(int id)
}
connect(_timer_, SIGNAL(timeout()),
this, SLOT(clearMessage()));
+ connect(_stats_timer_, SIGNAL(timeout()),
+ this, SLOT(showStats()));
+ d.statusbar_stats_timer_.start(d.timer_rate);
// We don't want to keep the window in memory if it is closed.
setAttribute(Qt::WA_DeleteOnClose, true);
@@ -627,6 +643,13 @@ GuiView::GuiView(int id)
busySVG, SLOT(hide()));
connect(busySVG,
Re: [patch] Selection stats in statusbar
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 even be decreased. If anyone wanted to do extra work, they could try to figure out how long the delay in Libre Office or Word is. I was more worried that there was unintended computation being done outside of the delayed computation so this addresses that as far as I'm concerned. > > I did not look, but will this be on by default? Would you object to a > > way to turn off the stats, like we have a way to turn off the zoom level > > and slider in the status bar? > > Already done. You can disable it in the same way you can disappear zoom > slider. Perfect, thanks! Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
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. > I did not look, but will this be on by default? Would you object to a > way to turn off the stats, like we have a way to turn off the zoom level > and slider in the status bar? Already done. You can disable it in the same way you can disappear zoom slider. Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Sun, Jul 31, 2022 at 12:35:30AM +0200, Pavel Sanda wrote: > On Fri, Jul 15, 2022 at 12:05:25AM +0200, Pavel Sanda wrote: > > On Thu, Jul 14, 2022 at 09:31:06AM +0200, Daniel wrote: > > > >The drawback is that's it's difficult to understand the interaction > > > >between > > > >the two timers now. I stared on the code for couple minutes and it was > > > >not clear to me what is the idea behind your stop/start changes. > > > > > > Some comments would probably have been good. I could try to add them if > > > there is interest. > > > > If I find little time I'll try the QTimer route and see whether we converge > > :) > > So I tried to find the best from solution from both worlds. > In the attached patch we use QTimer (0.5s between updates) for updating stats. > To avoid tricky interactions with another timer and current messages in > status bar I simply cretaed completely new label next to the slider, so it's > independent mechanism with no interactions. > > On top of that its possible to disable the visibility via context menu, but > it's > part of session not new RC variable, which seems good enough compromise. > > The update interval is now 0.5s, let me know if there is any slugishness > in your testing. I didn't see any, but I can easliy increase to 1s if need be. > > Any objections now? > > Pavel 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 not look, but will this be on by default? Would you object to a way to turn off the stats, like we have a way to turn off the zoom level and slider in the status bar? If you would object, I suggest we take a poll on lyx-users. I would vote against it (since I don't like numbers changing as I change the selection) but I have a feeling most users would be in favor of it, and I would be fine with that result. Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Fri, Jul 15, 2022 at 12:05:25AM +0200, Pavel Sanda wrote:
> On Thu, Jul 14, 2022 at 09:31:06AM +0200, Daniel wrote:
> > >The drawback is that's it's difficult to understand the interaction between
> > >the two timers now. I stared on the code for couple minutes and it was
> > >not clear to me what is the idea behind your stop/start changes.
> >
> > Some comments would probably have been good. I could try to add them if
> > there is interest.
>
> If I find little time I'll try the QTimer route and see whether we converge :)
So I tried to find the best from solution from both worlds.
In the attached patch we use QTimer (0.5s between updates) for updating stats.
To avoid tricky interactions with another timer and current messages in
status bar I simply cretaed completely new label next to the slider, so it's
independent mechanism with no interactions.
On top of that its possible to disable the visibility via context menu, but it's
part of session not new RC variable, which seems good enough compromise.
The update interval is now 0.5s, let me know if there is any slugishness
in your testing. I didn't see any, but I can easliy increase to 1s if need be.
Any objections now?
Pavel
diff --git a/lib/ui/stdcontext.inc b/lib/ui/stdcontext.inc
index bd94e7c904..8c79c5f351 100644
--- a/lib/ui/stdcontext.inc
+++ b/lib/ui/stdcontext.inc
@@ -731,6 +731,8 @@ Menuset
Separator
Item "Show Zoom Level|Z" "ui-toggle zoomlevel"
Item "Show Zoom Slider|S" "ui-toggle zoomslider"
+ Separator
+ Item "Show Document Statistics|D" "ui-toggle statistics"
End
End
diff --git a/src/LyXAction.cpp b/src/LyXAction.cpp
index 992b47a441..ae8245d070 100644
--- a/src/LyXAction.cpp
+++ b/src/LyXAction.cpp
@@ -4097,6 +4097,7 @@ void LyXAction::init()
frame : Toggle visibility of the frames around editing
window.\n
zoomslider : Toggle visibility of the zoom slider in
statusbar.\n
zoomlevel : Toggle visibility of the zoom level display in
statusbar.\n
+ statistics : Toggle visibility of the document statistics count
in statusbar.\n
fullscreen : Toggle fullscreen mode. This also covers calling
the
previous functions. However #LFUN_TOOLBAR_TOGGLE
for the
custom tweaks of the toolbars should be used.
diff --git a/src/frontends/qt/GuiView.cpp b/src/frontends/qt/GuiView.cpp
index 8f6bf07115..c381d547b5 100644
--- a/src/frontends/qt/GuiView.cpp
+++ b/src/frontends/qt/GuiView.cpp
@@ -523,6 +523,7 @@ public:
///
QTimer statusbar_timer_;
+ QTimer statusbar_stats_timer_;
/// auto-saving of buffers
Timeout autosave_timeout_;
@@ -553,8 +554,8 @@ QSet GuiView::GuiViewPrivate::busyBuffers;
GuiView::GuiView(int id)
: d(*new GuiViewPrivate(this)), id_(id), closing_(false), busy_(0),
- command_execute_(false), minibuffer_focus_(false),
toolbarsMovable_(true),
- devel_mode_(false)
+ command_execute_(false), minibuffer_focus_(false),
stat_counts_enabled_(true),
+ toolbarsMovable_(true), devel_mode_(false)
{
connect(this, SIGNAL(bufferViewChanged()),
this, SLOT(onBufferViewChanged()));
@@ -582,6 +583,9 @@ GuiView::GuiView(int id)
}
connect(_timer_, SIGNAL(timeout()),
this, SLOT(clearMessage()));
+ connect(_stats_timer_, SIGNAL(timeout()),
+ this, SLOT(showStats()));
+ d.statusbar_stats_timer_.start(1000);
// We don't want to keep the window in memory if it is closed.
setAttribute(Qt::WA_DeleteOnClose, true);
@@ -627,6 +631,13 @@ GuiView::GuiView(int id)
busySVG, SLOT(hide()));
connect(busySVG, SIGNAL(pressed()), this,
SLOT(checkCancelBackground()));
+ stat_counts_ = new QLabel(statusBar());
+ stat_counts_->setAlignment(Qt::AlignCenter);
+ stat_counts_->setFrameStyle(QFrame::StyledPanel);
+ stat_counts_->hide();
+ statusBar()->addPermanentWidget(stat_counts_);
+
+
QFontMetrics const fm(statusBar()->fontMetrics());
zoom_slider_ = new QSlider(Qt::Horizontal, statusBar());
@@ -952,6 +963,7 @@ void GuiView::saveLayout() const
settings.setValue("icon_size", toqstr(d.iconSize(iconSize(;
settings.setValue("zoom_value_visible", zoom_value_->isVisible());
settings.setValue("zoom_slider_visible", zoom_slider_->isVisible());
+ settings.setValue("document_stats_enabled", stat_counts_enabled_);
}
@@ -1001,6 +1013,9 @@ bool GuiView::restoreLayout()
zoom_in_->setVisible(show_zoom_slider);
zoom_out_->setVisible(show_zoom_slider);
+ stat_counts_enabled_ = settings.value("document_stats_enabled",
true).toBool();
+ stat_counts_->setVisible(stat_counts_enabled_);
+
if (guiApp->platformName() == "qt4x11" ||
Re: [patch] Selection stats in statusbar
On Thu, Jul 14, 2022 at 09:31:06AM +0200, Daniel wrote: > >The drawback is that's it's difficult to understand the interaction between > >the two timers now. I stared on the code for couple minutes and it was > >not clear to me what is the idea behind your stop/start changes. > > Some comments would probably have been good. I could try to add them if > there is interest. If I find little time I'll try the QTimer route and see whether we converge :) Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 2022-07-14 00:13, Pavel Sanda wrote: On Wed, Jul 13, 2022 at 08:54:28AM +0200, Daniel wrote: Hi, There is also a patch for instant counts at https://www.lyx.org/trac/ticket/12422. It works with delayed calculations. I have no idea how this does performance wise, but it might be worth checking out. Yes, that's lazy man approach, using timer instead of parallel thread. I got the idea when I noticed that other word processors (e.g. LO Writer, MS Word) showed the stats delayed. So, I thought that this is probably (part of) a decent way to implement this. The advantage is that you avoid race-conditions (though I think you miss stop() inside closeEvent()?). I might have missed that. It's some time ago that I wrote the code and don't remember from the top of my head. The drawback is that's it's difficult to understand the interaction between the two timers now. I stared on the code for couple minutes and it was not clear to me what is the idea behind your stop/start changes. Some comments would probably have been good. I could try to add them if there is interest. JMarc's warning about ineeficiency of counting remain unaddressed, though it migh not be visible because of 300ms update sampling frequency. Yes, I just took for granted the stats that LyX already generates. My patch inherits all the downsides of whatever approach is used there. Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Wed, Jul 13, 2022 at 08:54:28AM +0200, Daniel wrote: > Hi, > > There is also a patch for instant counts at > https://www.lyx.org/trac/ticket/12422. It works with delayed calculations. I > have no idea how this does performance wise, but it might be worth checking > out. Yes, that's lazy man approach, using timer instead of parallel thread. The advantage is that you avoid race-conditions (though I think you miss stop() inside closeEvent()?). The drawback is that's it's difficult to understand the interaction between the two timers now. I stared on the code for couple minutes and it was not clear to me what is the idea behind your stop/start changes. JMarc's warning about ineeficiency of counting remain unaddressed, though it migh not be visible because of 300ms update sampling frequency. Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On 2022-07-13 00:50, Pavel Sanda wrote: On Tue, Jul 12, 2022 at 04:31:20PM -0400, Scott Kostyshak wrote: However, it seems to have a pretty big hit on performance. The way I checked was starting with select-all on a document, and then doing shift + up and seeing how long it took to get to the top. It took 16 seconds without the patch and 47 seconds with the patch. I don't have a good argument for a use case for this particular stress test, but it seems to raise a flag, and the document I used to test is not too long (it compiles to a 14 page PDF). I see. Admittedly I used it only localy for couple paragraphs and it did not occur to me to try selection of the whole document. In a local use even on my super-speedy repeat rate of keyboard nothing is noticeable but when I select complete User Guide the lag is indeed visible. One could perhaps disable the feature if (to-from) is too big. I think it would be nice to add an option to show/remove it. As you've guessed I am no fan of new options for details like this. If that doesn't work, would this be a good candidate for asynchronous calculation? This would be indeed an option. We could show statistics for the whole document or just selection when selected and update it in a parallel thread once in a second or so. I have a feeling that MS Word is doing something like this based on it's laggy updates of statusbar. I know I'm asking to do a lot of work You are and I am nowhere close to having time for that :) As such this change goes to my local patchset and I am rectracting the proposal... Pavel Hi, There is also a patch for instant counts at https://www.lyx.org/trac/ticket/12422. It works with delayed calculations. I have no idea how this does performance wise, but it might be worth checking out. Daniel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Tue, Jul 12, 2022 at 04:31:20PM -0400, Scott Kostyshak wrote: > However, it seems to have a pretty big hit on performance. The way I > checked was starting with select-all on a document, and then doing shift > + up and seeing how long it took to get to the top. It took 16 seconds > without the patch and 47 seconds with the patch. I don't have a good > argument for a use case for this particular stress test, but it seems to > raise a flag, and the document I used to test is not too long (it > compiles to a 14 page PDF). I see. Admittedly I used it only localy for couple paragraphs and it did not occur to me to try selection of the whole document. In a local use even on my super-speedy repeat rate of keyboard nothing is noticeable but when I select complete User Guide the lag is indeed visible. One could perhaps disable the feature if (to-from) is too big. > I think it would be nice to add an option to show/remove it. As you've guessed I am no fan of new options for details like this. > If that doesn't work, would this be a good candidate for asynchronous > calculation? This would be indeed an option. We could show statistics for the whole document or just selection when selected and update it in a parallel thread once in a second or so. I have a feeling that MS Word is doing something like this based on it's laggy updates of statusbar. > I know I'm asking to do a lot of work You are and I am nowhere close to having time for that :) As such this change goes to my local patchset and I am rectracting the proposal... Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Le 12/07/2022 à 22:31, Scott Kostyshak a écrit : Part of it is that I just like to bother JMarc, I am glad to humor you. As for how to solve the performance issue, is there any smart caching that can be done? The first thing to do would be to scrap the current method and create a new version from scratch. Going though a document with an iterator character by character is a horribly slow by design. The code goes though each and every math formula without any hope of finding something. updateBuffer is a good example of how to do this right. JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
On Tue, Jul 12, 2022 at 09:12:56PM +0200, Pavel Sanda wrote: > Hi, > > I need to adhere precise word/character counts in sections of a document. > For this I need to have instantaneous visual feedback of stats when selecting > blocks of text. > > The attached patch does this. Is there some objection to push this into > master? > (If no objection raised I'll add string translation layer to the patch.) Tested and works well. However, it seems to have a pretty big hit on performance. The way I checked was starting with select-all on a document, and then doing shift + up and seeing how long it took to get to the top. It took 16 seconds without the patch and 47 seconds with the patch. I don't have a good argument for a use case for this particular stress test, but it seems to raise a flag, and the document I used to test is not too long (it compiles to a 14 page PDF). That said, I feel like a considerable amount of users would like this patch. I would not, but I think it's because my eyes are sensitive. You can ask JMarc about all of the painting-related issues I've bothered him about that no one else reported. Part of it is that I just like to bother JMarc, but I also recognize that I don't like things changing on the screen and I'm not typical in this sense. If you can solve the performance issues, I think it would be nice to add an option to show/remove it. I know there is pushback against yet-another-preference, but this could go in the context menu of the status bar. Already we have entries there for "Show Zoom Level" and "Show Zoom Slider" (which I also like to turn off). As for how to solve the performance issue, is there any smart caching that can be done? For example, when selection changes, can you use the old statistics and then just update the statistics based on the difference? If this were done, the initial "select-all" would still have a performance hit, but at least when changing the selection it should not be too bad. Alternatively, can you store the statistics by paragraph, and then assuming it is cheap to check if a paragraph has changed since the last calculation, you can just update the statistics of the changed paragraphs? If that doesn't work, would this be a good candidate for asynchronous calculation? e.g., do the work up to 'from' and 'to' synchronously. Then do the rest asynchronously. After buf.updateStatistics() is done, you can check whether: (1) there is still a selection (2) 'from' is still the same as cur.selectionBegin() (3) 'to' is still the same as cur.selectionEnd() If those checks hold, then write the status. I know I'm asking to do a lot of work, and this is opening up a big can of worms for other problems. I might be able to help someday with the asynchronous design if indeed the caching does not work for some reason and if there is support from others for it. I've been meaning to study the latest C++ features anyway whenever I can find the time. I guess the first step would be: if this will go into 2.5.0 (because of the asynchronous design), what C++ standard will we require for it? To be clear, I don't know *when* I could work on this, but we could make a plan for you to use your patch locally in the meantime and whenever I can (maybe in a year) I will work on the asynchronous part. Or if anyone else is interested, they would be welcome :) Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: [patch] Selection stats in statusbar
Le 12/07/2022 à 21:12, Pavel Sanda a écrit : Hi, I need to adhere precise word/character counts in sections of a document. For this I need to have instantaneous visual feedback of stats when selecting blocks of text. The attached patch does this. Is there some objection to push this into master? (If no objection raised I'll add string translation layer to the patch.) What is the performance cost? JMarc -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
[patch] Selection stats in statusbar
Hi,
I need to adhere precise word/character counts in sections of a document.
For this I need to have instantaneous visual feedback of stats when selecting
blocks of text.
The attached patch does this. Is there some objection to push this into master?
(If no objection raised I'll add string translation layer to the patch.)
Pavel
diff --git a/src/Text.cpp b/src/Text.cpp
index 7857e5e06c..6a7b80d93f 100644
--- a/src/Text.cpp
+++ b/src/Text.cpp
@@ -2146,6 +2146,18 @@ docstring Text::currentState(CursorData const & cur,
bool devel_mode) const
}
}
+ // Stats if selection is done
+ if (cur.selection()) {
+ DocIterator from, to;
+ from = cur.selectionBegin();
+ to = cur.selectionEnd();
+ buf.updateStatistics(from, to);
+ int const words = buf.wordCount();
+ int const chars = buf.charCount(false);
+ int const chars_blanks = buf.charCount(true);
+ os << ", w:" << words << " c:" << chars << " cb:"<--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
