Review Request 111968: Make newly opened search tabs close- and editable

[Frank Reininghaus]

> On March 24, 2014, 11:45 p.m., Frank Reininghaus wrote:
> > Thanks for the updated patch, and sorry for the late reply. Looks good so far, and I can confirm that it works. However, if I replace your step 2 by "split the view" and then immediately go to step 4, i.e., close the search in the new view, both searches are closed. Can you confirm that?
> > 
> > Concerning the names of the new methods: I'm not sure if "fromRatingTerm" and "fromFacetTypes" make it clear what these methods do. Maybe "set" would be slightly better than "from"?
> 
> Emmanuel Pescosta wrote:
>     Sounds like bug 315904?
>     
>     > I'm not sure if "fromRatingTerm" and "fromFacetTypes" make it clear what these methods do. Maybe "set" would be slightly better than "from"?
>     Really? To*/from* methods are commonly used for such cases. I think from* is better in this specific case, 
>     because we do not only set values but also extract information first (which is unusual for setters). ;)

> Sounds like bug 315904?

Oh yes, thanks. Even though the number of open bug reports has decreased considerably during the past year, I'm still unable to remember every one of them.

>> I'm not sure if "fromRatingTerm" and "fromFacetTypes" make it clear what these methods do. Maybe "set" would be slightly better than "from"?
> Really? To*/from* methods are commonly used for such cases. I think from* is better in this specific case, 
> because we do not only set values but also extract information first (which is unusual for setters). ;)

I had thought that from* functions are mostly functions that convert something to another format, like

QString s = QString::fromUtf8(foo)
QByteArray b = QByteArray::fromHex("0xff")

But maybe it's just because I haven't seen any other use of from* functions yet. In any case, it's a minor detail that's probably not worth discussing much about.


- Frank


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


On March 25, 2014, 10:35 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/111968/
> -----------------------------------------------------------
> 
> (Updated March 25, 2014, 10:35 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 311950
>     http://bugs.kde.org/show_bug.cgi?id=311950
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> When you open a new tab while the search mode is enabled, the newly opened 
> tab also starts the same search (Because new tab is opened with the current 
> view url), but the search box is in read-only mode. 
> So you cannot close the search bar nor edit the search text.
> 
> This patch fixes this by parsing the search url. The value of the "search"
> parameter is used as search text and the value of the "url" parameter is used 
> for the search path ("root" folder for the search when "Search from here" mode 
> is enabled).
> 
> In case of Baloo search urls, we use Baloo::Query::fromSearchUrl.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinviewcontainer.cpp 768fd5e 
>   dolphin/src/search/dolphinfacetswidget.h 1fd1683 
>   dolphin/src/search/dolphinfacetswidget.cpp 3824eb9 
>   dolphin/src/search/dolphinsearchbox.h 6dc09f1 
>   dolphin/src/search/dolphinsearchbox.cpp 0a70566 
> 
> Diff: https://git.reviewboard.kde.org/r/111968/diff/
> 
> 
> Testing
> -------
> 
> 1. Start a search (e.g. "kde") in folder $HOME/Downloads
> 2. Open a new tab
> 3. Edit the search mode should work
> 4. Close the search
> 5. The view from the newly opened tab should open $HOME/Downloads
> 
> All five points works for me.
> 
> Tested with Baloo 4.13 Beta1
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140326/299c602a/attachment.htm>