Review Request 119243: Better OSX integration: native file dialogs and unified title/toolbar

[Thomas Lübking]

> On Juli 12, 2014, 2:11 nachm., Aleix Pol Gonzalez wrote:
> > kio/kfile/kfiledialog.cpp, line 316
> > <https://git.reviewboard.kde.org/r/119243/diff/1/?file=289740#file289740line316>
> >
> >     I don't know why you did that, but it doesn't look good.
> 
> Marko Käning wrote:
>     Actually, when submitting this RR I was also stumbling over this commented out line and wondered why...
>     
>     I am afraid we've to wait for René to figure this out.
> 
> Ian Wadham wrote:
>     If we are going to use Review Board, can we get the kde-mac list added as a group? How do you do that?
>     
>     That would let René, Boudewijn and Nicolas listen in on this stuff, FWIW.
> 
> Ian Wadham wrote:
>     Actually I do not think Review Board is worth much.  It is extra work, hard to use and, in my experience, rarely results in anything much more than white-space adjustments - as you can see from the unanswered questions, issues and comments above, by Mark Gaiser, Marko and me.  Actually I answered Marko's issue, but I would far rather be doing so, more quickly and directly, on kde-mac or BKO, if none of the kdelibs maintainers have anything useful to add.  Nevertheless I will have one more try at reviewing this patch, now that I have tested it on Apple OS X with a few different file-open and save situations.

You'd need to request addition of the group to groups (just a formality to add the entry, ask sysadmin æt kde döt org), otherwise you've to add rene (rjvbb) directly.

The file dialog apparently requires the native fallback in case the start dir is not local.
The string UnifiedTitleAndToolBarOnMac is only used here: http://lxr.kde.org/search?_filestring=&_string=UnifiedTitleAndToolBarOnMac 
The key is "Native" in kdeglobals (~/.kde/share/config/kdeglobals), [KFileDialog Settings] group (just looked up the code on the static flag, see http://api.kde.org/4.x-api/kdelibs-apidocs/kio/html/kfiledialog_8cpp_source.html from line 206)

A very specific problem about this review is that the submitter is not the author.

The worth of reviewboard is (at least) to elimintate bad commits (see "commented d->w->setCustomWidget(QString(), customWidget);" - whatever the problem is, this solution not only adds cruft and lacks a comment but most certainly introduces a bug, since it basically kills the API feature)

... oh, and this review was opened on the FIFA cup finals weekend - eg. i still won't be near a usable workstation before tonight. And I think it's amazing™ that i can already use a keyboard again =)


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119243/#review62184
-----------------------------------------------------------


On Juli 12, 2014, 9:55 vorm., Marko Käning wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119243/
> -----------------------------------------------------------
> 
> (Updated Juli 12, 2014, 9:55 vorm.)
> 
> 
> Review request for kdelibs, Christoph Feck and Ian Wadham.
> 
> 
> Bugs: 337356
>     http://bugs.kde.org/show_bug.cgi?id=337356
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This bundles both patches submitted by René J.V. Bertin in the associated issue
> 
> 
> Diffs
> -----
> 
>   kdeui/widgets/kmainwindow.cpp 85beaccdb6f123d2996b8c2b5e38435265c63ff8 
>   kio/kfile/kfiledialog.h 2b11796897ff095279e7c254a383a9e8e323ea0f 
>   kio/kfile/kfiledialog.cpp 4005ba304a18b59572cc1aece3fcd122ce05fda9 
> 
> Diff: https://git.reviewboard.kde.org/r/119243/diff/
> 
> 
> Testing
> -------
> 
> See issue for more info from René.
> 
> ---
> 
> I myself haven't yet tested this. Will report back as soon as I got to it.
> 
> 
> Thanks,
> 
> Marko Käning
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140714/9c562043/attachment.htm>