Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[coreSubs] option to manually entering search string #3816

Merged
merged 1 commit into from

4 participants

@amet

apparently manual search string option is needed in subs search/download, dont ask me why but I think its to do with incorrectly named files or files that are not scanned into library.

I have left out the setting to enable this, we dont need more settings in Subtitle section, and doesnt hurt to have this listed all the time.

if anyone wants to test, https://github.com/amet/service.subtitles.opensubtitles/tree/ManualSearch has the changes

@jmarshallnz
Owner

You're adding the manual item to the services menu? But then when the user selects it, you're doing a search using the already selected item?

Perhaps instead it should be in the subtitles list?

@amet

@jmarshallnz yes, it's added to service list and when user selects it it uses previously used service and entered search string to try to find subs.

As mentioned, services will need adapting for it if they want to support it.

Not sure why would it be in subtitles list, but it should not be difficult to get it there instead.

@jmarshallnz
Owner

I just dunno if it's obvious that the "Manual search" uses the currently selected service, as it's listed as if it's a service in it's own right.

Does it make more sense to just use a separate button below the service list?

@amet

We could also do that ... If plan is to have it in Gotham I'll work on it next week

@jmarshallnz
Owner
@amet

@jmarshallnz updated to have a button below the services list. maybe @ronie has better plan for the skin part of it.

@ronie
Collaborator

maybe @ronie has better plan for the skin part of it.

no need I think... it looks beautiful :)

@jmarshallnz
Owner

@amet: mind squashing down?

@amet

@jmarshallnz done, also fixed the failure if dialog is exited without entering the search string, in that case we didnt have "action" at all. we now set action to search and if its manual search we set it again. is that bad practice?

@ronie sorry to claim your commit, needed to squash it. ALL SKIN CHANGES CREDIT TO @ronie < - is that enough? ;p

@ronie
Collaborator

'forever in debt to your priceless skills' would work for me ;-)

xbmc/video/dialogs/GUIDialogSubtitles.cpp
@@ -305,6 +313,14 @@ void CGUIDialogSubtitles::Search()
if (setting)
url.SetOption("languages", setting->ToString());
+ if (manual)
+ {
+ if (CGUIKeyboardFactory::ShowAndGetInput(m_strManualSearch, g_localizeStrings.Get(24121), true))
+ {
+ url.SetOption("action", "manualsearch");
+ url.SetOption("searchstring", m_strManualSearch);
+ }
+ }
@jmarshallnz Owner

2 changes needed I think:

  1. m_strManualSearch isn't needed - you can just use a temporary string here. Or are you saving it here so that the next search can use it in case the first returned no results?

  2. I think it'd be tidier if the keyboard stuff was done outside Search(), e.g. in the OnMessage() handler, with the string to search passed into Search() instead of the boolean. Then you don't need the two url.SetOptions (not that they're a problem) as you can just do:

if (!search.empty())
{
  url.SetOption("action", "manualsearch");
  url.SetOption("searchstring", search);
}
else
  url.SetOption("action", "search");
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@amet

@jmarshallnz

  1. yes, I need to keep m_strManualSearch for any later search, I was even thinking to reset it on playback start that assumes the movie changed. currently it will not change until xbmc restart

  2. assuming you are ok with 1. above, search.empty() will become m_strManualSearch.empty() and it will always be something. unless we find a way to clear it.

@jmarshallnz
Owner
  1. Fine.

  2. Nope. Change Search(bool manual = false) to Search(const std::string &search = ""). Call it as Search(m_strManualSearch); from the manual search handler, after doing the keyboard stuff first there.

@amet

@jmarshallnz but since m_strManualSearch is always something on the second run, above if (!search.empty()) is never going to be false. and it will keep calling manual search. or am I misunderstanding this?

@jmarshallnz
Owner

In OnMessage() handler for the manual search button you do the keyboard stuff, updating m_strManualSearch. You then call Search(m_strManualSearch). In the OnMessage() handler for the normal service click, you just call Search(). i.e. Search should not be doing keyboard stuff - it should simply take a string. If empty, it's a normal search. If not empty, it's the manual search with that search string.

@amet

@jmarshallnz ok, i get what you mean ... i'll fix it up later today

@amet

@jmarshallnz done, please check

@jmarshallnz
Owner

jenkins build this please

@jmarshallnz jmarshallnz merged commit a701a9e into from
@amet amet deleted the branch
@Mafarricos

In latest February released manualsearch doesn't work, there was a change in this manual search?

@amet

whats skin are you using? always use confluence when testing new features

"... manualsearch doesn't work..." is not a good description of the problem you have. Use forum to discuss this further please

@Mafarricos

I installed a fresh XBMC, so I'm using the skin provided with the installation.
I installed the nightly from yesterday for windows, and when you click in the manual search (in the subtitles addon) nothing happens, also tried some nightlies to raspberry pi, only the latest releases from January work with manual search for subtitles. None of the February releases work. Is this a function that is going to be deprecated? Or is a bug that still wasn't reported.

I already stated this in the forum, but as I didn't had any response, I thought that here also could be used to explain the problem, and understand if was a change in this functionality or a XBMC core problem.

@amet

as far as I know, only service that supports manual search is opensubtitles.

try it and post a debug log of it not working in the forum and we can look into it.

one more time.... use forum to discuss it, no more responses from me here

@Mafarricos

OK, no problem.
I'm making log file in the version that works, and will make the log in the version that doesn't work and will make the post in the forum.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
24 addons/skin.confluence/720p/DialogSubtitles.xml
@@ -8,7 +8,7 @@
<controls>
<control type="group" id="250">
<animation effect="slide" start="0,0" end="900,0" time="500" tween="quadratic" easing="out">WindowClose</animation>
- <animation type="Conditional" condition="Control.HasFocus(150)" reversible="true">
+ <animation type="Conditional" condition="Control.HasFocus(150) | Control.HasFocus(160)" reversible="true">
<effect type="slide" end="-250,0" time="400"/>
</animation>
<control type="button" id="8999">
@@ -350,11 +350,11 @@
<left>900</left>
<top>206</top>
<width>250</width>
- <height>434</height>
+ <height>399</height>
<onleft>120</onleft>
<onright>120</onright>
- <onup>150</onup>
- <ondown>150</ondown>
+ <onup>160</onup>
+ <ondown>160</ondown>
<viewtype label="535">list</viewtype>
<scrolltime>200</scrolltime>
<itemlayout width="400" height="36">
@@ -408,6 +408,22 @@
</control>
</focusedlayout>
</control>
+ <control type="button" id="160">
+ <description>Manual search button</description>
+ <left>925</left>
+ <top>640</top>
+ <width>200</width>
+ <height>40</height>
+ <onleft>120</onleft>
+ <onright>120</onright>
+ <onup>150</onup>
+ <ondown>150</ondown>
+ <label>$LOCALIZE[24120]</label>
+ <font>font12_title</font>
+ <textcolor>white</textcolor>
+ <focusedcolor>white</focusedcolor>
+ <align>center</align>
+ </control>
</control>
</control>
</controls>
View
12 language/English/strings.po
@@ -12082,7 +12082,17 @@ msgctxt "#24119"
msgid "Select service that will be used as default to search for Movie subtitles"
msgstr ""
-#empty strings from id 24120 to 24999
+#: xbmc/dialogs/GUIDialogSubtitles.cpp
+msgctxt "#24120"
+msgid "Manual search string"
+msgstr ""
+
+#: xbmc/dialogs/GUIDialogSubtitles.cpp
+msgctxt "#24121"
+msgid "Enter search string"
+msgstr ""
+
+#empty strings from id 24121 to 24999
msgctxt "#25000"
msgid "Notifications"
View
23 xbmc/video/dialogs/GUIDialogSubtitles.cpp
@@ -30,6 +30,7 @@
#include "filesystem/PluginDirectory.h"
#include "filesystem/SpecialProtocol.h"
#include "guilib/GUIImage.h"
+#include "guilib/GUIKeyboardFactory.h"
#include "settings/MediaSettings.h"
#include "settings/Settings.h"
#include "settings/VideoSettings.h"
@@ -52,6 +53,7 @@ using namespace XFILE;
#define CONTROL_SUBSEXIST 130
#define CONTROL_SUBSTATUS 140
#define CONTROL_SERVICELIST 150
+#define CONTROL_MANUALSEARCH 160
/*! \brief simple job to retrieve a directory and store a string (language)
*/
@@ -138,6 +140,15 @@ bool CGUIDialogSubtitles::OnMessage(CGUIMessage& message)
return true;
}
+ else if (iControl == CONTROL_MANUALSEARCH)
+ {
+ //manual search
+ if (CGUIKeyboardFactory::ShowAndGetInput(m_strManualSearch, g_localizeStrings.Get(24121), true))
+ {
+ Search(m_strManualSearch);
+ return true;
+ }
+ }
}
else if (message.GetMessage() == GUI_MSG_WINDOW_DEINIT)
{
@@ -290,7 +301,7 @@ const CFileItemPtr CGUIDialogSubtitles::GetService() const
return CFileItemPtr();
}
-void CGUIDialogSubtitles::Search()
+void CGUIDialogSubtitles::Search(const std::string &search/*=""*/)
{
if (m_currentService.empty())
return; // no services available
@@ -299,7 +310,13 @@ void CGUIDialogSubtitles::Search()
ClearSubtitles();
CURL url("plugin://" + m_currentService + "/");
- url.SetOption("action", "search");
+ if (!search.empty())
+ {
+ url.SetOption("action", "manualsearch");
+ url.SetOption("searchstring", search);
+ }
+ else
+ url.SetOption("action", "search");
const CSetting *setting = CSettings::Get().GetSetting("subtitles.languages");
if (setting)
@@ -313,7 +330,7 @@ void CGUIDialogSubtitles::OnJobComplete(unsigned int jobID, bool success, CJob *
const CURL &url = ((CSubtitlesJob *)job)->GetURL();
const CFileItemList *items = ((CSubtitlesJob *)job)->GetItems();
const std::string &language = ((CSubtitlesJob *)job)->GetLanguage();
- if (url.GetOption("action") == "search")
+ if (url.GetOption("action") == "search" || url.GetOption("action") == "manualsearch")
OnSearchComplete(items);
else
OnDownloadComplete(items, language);
View
3  xbmc/video/dialogs/GUIDialogSubtitles.h
@@ -49,7 +49,7 @@ class CGUIDialogSubtitles : public CGUIDialog, CJobQueue
enum STATUS { NO_SERVICES = 0, SEARCHING, SEARCH_COMPLETE, DOWNLOADING };
void UpdateStatus(STATUS status);
- void Search();
+ void Search(const std::string &search="");
void OnSearchComplete(const CFileItemList *items);
void Download(const CFileItem &subtitle);
@@ -62,6 +62,7 @@ class CGUIDialogSubtitles : public CGUIDialog, CJobQueue
CFileItemList* m_serviceItems;
std::string m_currentService;
std::string m_status;
+ CStdString m_strManualSearch;
bool m_pausedOnRun;
bool m_updateSubsList; ///< true if we need to update our subs list
};
Something went wrong with that request. Please try again.