Hi Jenei
On 08/08/11 11:06, Jenei Gábor wrote:
Hello,
I would like to inform everyone that I attached my purposed patch that
fixes the bug 39168 in our bug report page, since it causes an already
mentioned problem in the overwriting dialog's popup I don't consider
it as something stable,
[...]
Here I also attach the purposed patch and I am waiting for your reviews.
well I have to admit I am not familiar with this hybrid/pdf thing, not
sure if it really is a good or bad thing, personally I really don't like
the double extension thing, but anyway I just have some general comments
on the patch
sfx2/source/dialog/filedlghelper.cxx
====================================
the first hunk in this patch is confusing, it changes the prevailing style of
positioning for '{'& '}' if there is no good reason to do that then it is
better not to make such a change
the second hunk that changes the logic of FileDialogHelper::GetPath is a little
worrying ( 'cause I guess this is used by many clients ), why was this
necessary and how does it change the existing behaviour?
sfx2/source/doc/guisaveas.cxx
=============================
2nd hunk is just whitespace change and again it's quite noisy to review
- in 'sal_Bool ModelData_Impl::OutputFileDialog'
+if(bAddStream==sal_True)
the equality check isn't really necessary&
if((bAddStream)
just reads better for me,
- please don't change the prevailing style for '{' / '}'
- using a map to store and retrieve the desired extension based on the
filtername would be neater and less cluttered that then the it/then construct
used in the patch.
what's really uncomfortable is ( from a quick apply& try of the patch ) is the
fact that the file name you are saving to is sort-of hijacked under the hood. e.g.
the file dialogs indicate one thing ( save with.pdf extension ) but do something
else ( save with a .odt.pdf extension ).
Using a new filter type would seem a more natural solution e.g. a filter type
that you could select in the 'save-as' dialog that would save the pdf in the
required hybrid format and would indicate the ( imo horrible dual extension )
The same filter type would be preselected ( if hybrid was selected in the pdf
export options dialog ) and shown file dialog raised by the export button. But
then again I only have a very passing familiarity with stuff, maybe someone
else might have more constructive advice
Noel
Noel
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
