[PATCH] fdo#34772 Word count / statistics in the status bar

classic Classic list List threaded Threaded
17 messages Options
Muhammad Haggag Muhammad Haggag
Reply | Threaded
Open this post in threaded view
|

[PATCH] fdo#34772 Word count / statistics in the status bar

Hello.

(Ux folks, please advise on the "UX/Localization" section below).

Bug:
====
https://bugs.freedesktop.org/show_bug.cgi?id=34772

Patch:
=====
https://bugs.freedesktop.org/attachment.cgi?id=61962

Patch review:
===========
https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=34772&attachment=61962

Changes (copied from patch):
========================
This change adds a new status bar control,
"SwWordCountStatusBarControl". The control shows the number of words
selected as well as the number of words in the whole document.
Double-clicking the control launches the modeless word count dialog.

Verification:
==========
I tested with small documents (to the tune of 30 pages, ~3500 words).
I'd like to try it out on a large/huge document to make sure there's
no performance impact. Are there any standard "large" documents used
for LO performance testing?

UX/Localization:
=============
Currently the status bar displays counts in the form: "Words:
<number-of-selected-words>/<total-number-of-words>". It's not clear at
a glance what the numbers mean. It looks like this:
http://imgur.com/3WqVD

How do you suggest we go about displaying this information clearly,
hopefully without being overly verbose?

Also, I'm not sure how localizable string literals are handled in the
code base. Does LO use something like gettext, or do I have to
explicitly add a string resource and load it?

Open Issues/Questions:
====================
* What's the point of viewsh.sdi? (compared to _viewsh.sdi).
_viewsh.sdi seems to be referencing the methods actually getting
invoked for word-count related logic.

* swriter.sdi: I based the entry for StateWordCount on StatePageCount.
I'm not sure what the fields mean (e.g. StatusBarConfig).

* How do I update LO help with material for the new status bar field?
I can see that other controls have their own documentation pages (e.g.
helpcontent2/source/text/swriter/02/08010000.xhp documents the page
count control), but I don't know what template the files follow or
what the fields mean. This is what I have so far:
https://bugs.freedesktop.org/attachment.cgi?id=61963

Regards,
--Muhammad
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Regina Henschel Regina Henschel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

Hi Muhammad,

Muhammad Haggag schrieb:
> Hello.
>
[..]
> Verification:
> ==========
> I tested with small documents (to the tune of 30 pages, ~3500 words).
> I'd like to try it out on a large/huge document to make sure there's
> no performance impact. Are there any standard "large" documents used
> for LO performance testing?

I don't know about "standard 'large' documents" for testing. You can use
the document 'part1' of the ODF specification, which has 864 pages.
http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.odt

Kind regards
Regina

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Muhammad Haggag Muhammad Haggag
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

On Tue, May 22, 2012 at 6:13 PM, Regina Henschel
<[hidden email]> wrote:
> Hi Muhammad,
>[...]
> I don't know about "standard 'large' documents" for testing. You can use the
> document 'part1' of the ODF specification, which has 864 pages.
> http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.odt
>
Thanks! That's exactly what I had in mind.

Aside from slow loading (due to having a debug + dbgutil build),
runtime performance seems unaffected compared to a vanilla build.

Regards,
--Muhammad
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Michael Meeks-2 Michael Meeks-2
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-ux-advise] [PATCH] fdo#34772 Word count / statistics in the status bar

In reply to this post by Muhammad Haggag

On Tue, 2012-05-22 at 18:36 +0300, Muhammad Haggag wrote:
> a glance what the numbers mean. It looks like this:
> http://imgur.com/3WqVD

        Wow - glad you got it working nicely :-) Thanks for that ...

> Also, I'm not sure how localizable string literals are handled in the
> code base. Does LO use something like gettext, or do I have to
> explicitly add a string resource and load it?

        You get to add the string into a '.src' file, and allocate a magic
number for it in a .hrc file (hunt for a spare space in the integer
space), then load it via a ResId()

        eg. aBuf.append(String(ScResId(SCSTR_EXTDOC_NOT_LOADED)));

        I'd git grep for STR_ in sc/ or something to see a number of those.

        Normally for this a pseudo-printf format would be used so use a % or $
magic in the string to be replaced. For an example of that:

String STR_RECOVER_QUERY
{
    Text [ en-US ] = "Should the file \"$1\" be restored?" ;
};

        Or somesuch (though I can't see that used - which is annoying, I wonder
why it is in the .src file and hence compiled, translated, and
distributed when it's not used ;-).

        Anyhow a quick hunt about for substituting that sort of thing would
prolly do what you want.

        Nice work !

                Michael.

--
[hidden email]  <><, Pseudo Engineer, itinerant idiot

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Petr Mladek Petr Mladek
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-ux-advise] [PATCH] fdo#34772 Word count / statistics in the status bar

Michael Meeks píše v Út 22. 05. 2012 v 17:57 +0100:
> Normally for this a pseudo-printf format would be used so use a % or $
> magic in the string to be replaced. For an example of that:
>
> String STR_RECOVER_QUERY
> {
>     Text [ en-US ] = "Should the file \"$1\" be restored?" ;
> };

You might find a working example of substitution in the commit
http://cgit.freedesktop.org/libreoffice/core/commit/?id=8c7ca43f690b12043d78a698f622eb565f305782

The important part is adding %ABOUTBOXPRODUCTVERSIONSUFFIX into
cui/source/dialogs/about.src and calling

rStr.SearchAndReplaceAllAscii( "%ABOUTBOXPRODUCTVERSIONSUFFIX",
rAboutBoxVersionSuffix );

in desktop/source/app/app.cxx


Note that this only describes how to modify existing string and how to
replace the value. If you want to add new GUI element, you need to
define also an ID in the related .hrc file, call ResId() to get the
localized string, ...


Best Regards,
Petr

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Muhammad Haggag Muhammad Haggag
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-ux-advise] [PATCH] fdo#34772 Word count / statistics in the status bar

Thanks for the help!

I created a followup patch that makes the strings as resources:
https://bugs.freedesktop.org/attachment.cgi?id=62063

Review:
https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=34772&attachment=62063

Regards,
--Muhammad
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Tor Lillqvist-2 Tor Lillqvist-2
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-ux-advise] [PATCH] [PUSHED] fdo#34772 Word count / statistics in the status bar

In reply to this post by Petr Mladek
Thanks, pushed (with the follow-up).

--tml
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Tor Lillqvist-2 Tor Lillqvist-2
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-ux-advise] [PATCH] [PUSHED] fdo#34772 Word count / statistics in the status bar

In reply to this post by Muhammad Haggag
Pushed the follow-up, too.

--tml
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Muhammad Haggag Muhammad Haggag
Reply | Threaded
Open this post in threaded view
|

Re: [Libreoffice-ux-advise] [PATCH] [PUSHED] fdo#34772 Word count / statistics in the status bar

I created a patch to update help content:
https://bugs.freedesktop.org/attachment.cgi?id=62112

Review:
https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=34772&attachment=62112

Regards,
--Muhammad
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

In reply to this post by Muhammad Haggag
Hi all,

this patch makes scrolling in writer impossible :( it always brings the
view back to the cursor (btw so does Tools -> Word Count, but only once
it is invoked).

It looks like the cause is the following lines:

@@ -1197,6 +1198,25 @@ void SwView::StateStatusLine(SfxItemSet &rSet)
                  }
              }
              break;
+
+            case FN_STAT_WORDCOUNT:
+            {
+                SwDocStat selectionStats;
+                SwDocStat documentStats;
+                {
+                    SwWait aWait( *GetDocShell(), sal_True );
+                    rShell.StartAction();
+                    rShell.CountWords(selectionStats);
+                    documentStats = rShell.GetUpdatedDocStat();
+                    rShell.EndAction();
+                }
+                rSet.Put(SfxStringItem(FN_STAT_WORDCOUNT,
rtl::OUStringBuffer("Words: ")
+
.append(rtl::OUString::valueOf(static_cast<sal_Int64>(selectionStats.nWord)))
+                                                            .append('/')
+
.append(rtl::OUString::valueOf(static_cast<sal_Int64>(documentStats.nWord))).makeStringAndClear()));
+            }
+            break;


Any idea? How to count words without changing a viewpoint?

Regards,
Ivan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

On 26.05.2012 22:06, Ivan Timofeev wrote:
> this patch makes scrolling in writer impossible :( it always brings the
> view back to the cursor

Oops, wait. That happens only if a document contains the Page Count
field (Insert -> Fields -> Page Count). WTF? Why it causes a recount
every second?...

So this is a bug in Writer, not in this patch... Will file it / poke
about in bugzilla.

Sorry,
Ivan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Muhammad Haggag Muhammad Haggag
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

On Sat, May 26, 2012 at 9:27 PM, Ivan Timofeev <[hidden email]> wrote:
> On 26.05.2012 22:06, Ivan Timofeev wrote:
>>
>> this patch makes scrolling in writer impossible :( it always brings the
>> view back to the cursor
>
>
> Oops, wait. That happens only if a document contains the Page Count field
> (Insert -> Fields -> Page Count). WTF? Why it causes a recount every
> second?...

I'm hazarding a guess that the page count field updates based on a
timer (say, every second), causing a state change, which causes a
recount. I didn't know that a recount causes the view to scroll to the
cursor, but it might be happening because of trying to count the
number of selected words.

> So this is a bug in Writer, not in this patch... Will file it / poke about
> in bugzilla.
>
Can you share the bug once you open it? I'd like to investigate this
in connection with word counting.

Regards,
--Muhammad
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

On 27.05.2012 12:28, Muhammad Haggag wrote:
> Can you share the bug once you open it?

Sure!

https://bugs.freedesktop.org/show_bug.cgi?id=50386

Cheers,
Ivan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Ivan Timofeev Ivan Timofeev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

Another nasty side-effect:

https://bugs.freedesktop.org/show_bug.cgi?id=50540

Who could have thought!
( *pensive* I hereby confirm that my claims like "This patch is safe"
are not valid anymore.)

Regards,
Ivan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Muhammad Haggag Muhammad Haggag
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

I have confirmed that the word counting logic somehow focuses on the cursor, and I'll either fix that or follow a model similar to the word count dialog where we update in response to editing actions only.

Unfortunately I'm currently on the road for a couple of days. I'll continue working on it on Saturday most likely. If the regressions are too bad we can revert the change until we have a fix.

Sorry for the trouble.

On May 31, 2012 4:29 PM, "Ivan Timofeev" <[hidden email]> wrote:
Another nasty side-effect:

https://bugs.freedesktop.org/show_bug.cgi?id=50540

Who could have thought!
( *pensive* I hereby confirm that my claims like "This patch is safe" are not valid anymore.)

Regards,
Ivan

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Jan Holesovsky Jan Holesovsky
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fdo#34772 Word count / statistics in the status bar

Hi Muhammad,

Muhammad Haggag píše v Čt 31. 05. 2012 v 21:46 +0200:

> I have confirmed that the word counting logic somehow focuses on the
> cursor, and I'll either fix that or follow a model similar to the word
> count dialog where we update in response to editing actions only.
>
> Unfortunately I'm currently on the road for a couple of days. I'll
> continue working on it on Saturday most likely. If the regressions are
> too bad we can revert the change until we have a fix.

I don't think we should revert that now, just before the feature
freeze :-)  Let's try to fix that during the beta phase, and revert only
if we don't manage to catch the bugs there.

Looking forward to the fixes! :-)

All the best,
Kendy

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Caolán McNamara Caolán McNamara
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] [PUSHED] fdo#34772 Word count / statistics in the status bar

In reply to this post by Muhammad Haggag
On Sat, 2012-05-26 at 01:21 +0300, Muhammad Haggag wrote:
> I created a patch to update help content:
> https://bugs.freedesktop.org/attachment.cgi?id=62112

I pushed the help documentation as well now.

C.

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice