Quantcast

[PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

classic Classic list List threaded Threaded
11 messages Options
Muhammad Haggag Muhammad Haggag
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

[PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

Hello,

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

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

Patch Review:
https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=37775&attachment=56740

Changes (copied from patch):
This patch changes LO behavior so that we update recent documents on
file open, save, save as, save all, and close. The previous behavior
was to only update the list on document close.

= Changes =
SfxPickList (sfxpicklist.cxx/hxx):
    . Extracted the logic to add a document to the "Recent Documents"
list into a function of its own: AddDocumentToPickList
        - Simplified the logic used by removing the check of
SfxObjectShell_impl::bWaitingForPickList (see
SfxObjectShell_impl::bWaitingForPickList below for details)
    . Modified SfxPickList::Notify to call the aforementioned function
on open, save, save-to, and save-as.

SfxObjectShell::APISaveAs_Impl (objserv.cxx):
    . Modified it to allow picklist entry when doing "Save As".

SfxObjectShell_impl::bWaitingForPickList (objstor.cxx, objxtor.cxx,
objshimp.hxx):
    . Removed this flag. It was used to indicate that a document wants
to be added to the picklist, then cleared after it's added. Since we
now always add documents to the picklist on saving, we no longer need
it.

= Verification =
The change is in sfx2, so it should apply to all LO apps. I verified
the new behavior in both writer and calc with the following actions:
    . File->Open
    . Open through File->Recent Documents->File Entry
    . File->Save
    . File->Save As
    . File->Save All
    . File->Close

I tested saving both odt and docx for writer.

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
|  
Report Content as Inappropriate
star

Re: [PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

On 08.02.2012 17:06, Muhammad Haggag wrote:

> Bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=37775
>
> Patch link:
> https://bugs.freedesktop.org/attachment.cgi?id=56740
>
> Patch Review:
> https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=37775&attachment=56740
>
> Changes (copied from patch):
> This patch changes LO behavior so that we update recent documents on
> file open, save, save as, save all, and close. The previous behavior
> was to only update the list on document close.

I added libreoffice-ux-advise to Cc...

@ux-team:
Do we really need such a feature? I mean, users don't want to open a
document from "Recent documents", if it is already opened, do they?

Best Regards,
Ivan
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Cor Nouws Cor Nouws
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

Hi Ivan, Muhammad,

Ivan Timofeev wrote (12-02-12 09:21)
> On 08.02.2012 17:06, Muhammad Haggag wrote:

>> This patch changes LO behavior so that we update recent documents on
>> file open, save, save as, save all, and close. The previous behavior
>> was to only update the list on document close.
>
> @ux-team:
> Do we really need such a feature? I mean, users don't want to open a
> document from "Recent documents", if it is already opened, do they?

Indeed, me thinks that the real improvement is the change of the recent
documents with all types of saving and closing. File Open does not look
useful to me.

--
  - Cor
  - http://nl.libreoffice.org

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

Re: [PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

Including Ivan and list again. Dropped them accidentally.

On Sun, Feb 12, 2012 at 2:12 PM, Muhammad Haggag <[hidden email]> wrote:
On Sun, Feb 12, 2012 at 12:16 PM, Cor Nouws <[hidden email]> wrote:
Ivan Timofeev wrote (12-02-12 09:21)
@ux-team:
Do we really need such a feature? I mean, users don't want to open a
document from "Recent documents", if it is already opened, do they?

Indeed, me thinks that the real improvement is the change of the recent documents with all types of saving and closing. File Open does not look useful to me.


It's not very useful, but it could be argued that it's logically sound--the "Recent Documents" list is updated whenever there's document activity, and "Open" is definitely a document activity. With the patch the way it is, opening a file that is already in the middle of the "Recent Documents" list will move it to the top of the list immediately. The reason this is not very useful is that "Close" would've updated it eventually, rendering it redundant except in the case of a LO crash. It's worth noting that this is what MS Word 2007 does, too (if we care about matching behavior).

If you're still not in favor, I can remove the update on Open and post another patch.

Regards,
--Muhammad


_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Korrawit Pruegsanusak Korrawit Pruegsanusak
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [Libreoffice-ux-advise] [PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

In reply to this post by Cor Nouws
Hello Cor, all,

On Sun, Feb 12, 2012 at 17:16, Cor Nouws <[hidden email]> wrote:
> Indeed, me thinks that the real improvement is the change of the recent
> documents with all types of saving and closing. File Open does not look
> useful to me.

IMHO the case which 'file open' would be useful is already described
in the bug comment 0 and 4: saving with a new name doesn't get old
name into the list.

But if it isn't renamed, could we place a check somewhere before
putting the file into the list?

Best Regards,
--
Korrawit Pruegsanusak
_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Cor Nouws Cor Nouws
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [Libreoffice-ux-advise] [PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

Korrawit Pruegsanusak wrote (12-02-12 13:30)
> IMHO the case which 'file open' would be useful is already described
> in the bug comment 0 and 4: saving with a new name doesn't get old
> name into the list.

Yes, if that is the technical reason, and can't be (easily) solved
different (as suggested below) I do not object to have it in!

> But if it isn't renamed, could we place a check somewhere before
> putting the file into the list?

Cheers,

--
  - Cor
  - http://nl.libreoffice.org

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

Re: [Libreoffice-ux-advise] [PATCH] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

On Sun, Feb 12, 2012 at 3:38 PM, Cor Nouws <[hidden email]> wrote:
Korrawit Pruegsanusak wrote (12-02-12 13:30)

IMHO the case which 'file open' would be useful is already described
in the bug comment 0 and 4: saving with a new name doesn't get old
name into the list.

Yes, if that is the technical reason, and can't be (easily) solved different (as suggested below) I do not object to have it in!


It's possible to solve that without having to update the list on open. We can intercept the beginning of the "Save As" operation and update the list with the old document. I uploaded a new patch that does that (and removed the list update on document open).

Patch:

Patch review:
 
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
|  
Report Content as Inappropriate
star

[PUSHED] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

Hi Muhammad,

On Tue, 2012-02-14 at 10:58 +0200, Muhammad Haggag wrote:
> It's possible to solve that without having to update the list on open.
> We can intercept the beginning of the "Save As" operation and update
> the list with the old document. I uploaded a new patch that does that
> (and removed the list update on document open).

        Incidentally, I really like the patch - and as someone who exits their
app often via either a SEGV, or an explicit kill, I'd really like the
recently used documents list to work ;-)

        I've merged it to master, thanks so much for your work here; do you
have another easy hack or some cleanup in your sights next ? :-) usually
things start to occur to people after they've read around the code for a
bit ;-)

        Thanks !

                Michael.

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

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

Re: [PUSHED] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

On Tue, Feb 14, 2012 at 6:31 PM, Michael Meeks <[hidden email]> wrote:
>
>        I've merged it to master, thanks so much for your work here; do you
> have another easy hack or some cleanup in your sights next ? :-) usually
> things start to occur to people after they've read around the code for a
> bit ;-)

Thanks for pushing :)
I'm currently investigating fdo#31005:
https://bugs.freedesktop.org/show_bug.cgi?id=31005
I think I'll be focusing on bug fixing (rather than doing cleanup) for
a while, and I'll probably be giving preference to old issues that
have been bugging people for years.

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
|  
Report Content as Inappropriate
star

Re: [PUSHED] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...


On Tue, 2012-02-14 at 18:44 +0200, Muhammad Haggag wrote:
> I'm currently investigating fdo#31005:
> https://bugs.freedesktop.org/show_bug.cgi?id=31005

        Let me read that :-)

> I think I'll be focusing on bug fixing (rather than doing cleanup) for
> a while, and I'll probably be giving preference to old issues that
> have been bugging people for years.

        Wow - you're my hero :-) fixing bugs is really, really useful work -
and there is never a shortage of them.

        Thanks !

                Michael.

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

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
Cor Nouws Cor Nouws
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate
star

Re: [PUSHED] fdo#37775 - EasyHack: Recent Documents not updated by Save & Save As...

In reply to this post by Michael Meeks-2
Michael Meeks wrote (14-02-12 17:31)

> Incidentally, I really like the patch - and as someone who exits their
> app often via either a SEGV, or an explicit kill, I'd really like the
> recently used documents list to work ;-)

Big thanks from me too.
It's one of those little annoyances that one gets sort of used to .. but
so much greater the joy when it's solved :-)

Cor

--
  - Cor
  - http://nl.libreoffice.org

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