Improve CITATION_OPEN

* The lyxpaperview script now only provides the paths and let us do the opening
* We use our own viewers for local files rather than QDesktopServices

Plus several minor improvements and code cleanup

lib/scripts/lyxpaperview.py

@@ -10,19 +10,10 @@
 # Full author contact details are available in file CREDITS
 
 # This script searches the home directory for a PDF or PS
-# file with a name containing year and author. If found,
-# it opens the file in a viewer. 
+# file with a name containing specific keywords (year and author by default).
+# If found, it returns the path(s), separated by \n.
 
-import getopt, os, sys, subprocess
-
-pdf_viewers = ('pdfview', 'kpdf', 'okular', 'qpdfview --unique',
-               'evince', 'xreader', 'kghostview', 'xpdf', 'SumatraPDF',
-               'acrobat', 'acroread', 'mupdf',
-               'gv', 'ghostview', 'AcroRd32', 'gsview64', 'gsview32')
-
-ps_viewers = ("kghostview", "okular", "qpdfview --unique",
-              "evince", "xreader", "gv", "ghostview -swap",
-              "gsview64", "gsview32")
+import os, sys, subprocess
 
 def message(message):
     sys.stderr.write("lyxpaperview: %s\n" % message)
@@ -32,7 +23,7 @@ def error(message):
     exit(1)
 
 def usage(prog_name):
-    msg = "Usage: %s [-v pdfviewer] [-w psviewer] titletoken-1 [titletoken-2] ... [titletoken-n]\n" \
+    msg = "Usage: %s titletoken-1 [titletoken-2] ... [titletoken-n]\n" \
           "    Each title token must occur in the filename (at an arbitrary position).\n" \
           "    You might use quotes to enter multi-word tokens"
     return  msg % prog_name
@@ -66,13 +57,6 @@ def find_exe(candidates):
     return None
 
 
-def find_exe_or_terminate(candidates):
-    exe = find_exe(candidates)
-    if exe == None:
-        error("Unable to find executable from '%s'" % " ".join(candidates))
-
-    return exe
-
 def find(args, path):
     if os.name != 'nt':
         # use locate if possible (faster)
@@ -84,10 +68,9 @@ def find(args, path):
                    # have this already
                    continue
                px = subprocess.Popen(['grep', '-i', arg], stdin=px.stdout, stdout=subprocess.PIPE)
-            p4 = subprocess.Popen(['head', '-n 1'], stdin=px.stdout, stdout=subprocess.PIPE)
             p1.stdout.close()
-            output = p4.communicate()
-            return output[0].decode("utf8")[:-1]# strip trailing '\n'
+            output = px.communicate()
+            return output[0].decode("utf8").strip('\n')
      # FIXME add something for windows as well?
      # Maybe dir /s /b %WINDIR%\*author* | findstr .*year.*\."ps pdf"
 
@@ -107,44 +90,14 @@ def find(args, path):
 def main(argv):
     progname = argv[0]
     
-    opts, args = getopt.getopt(sys.argv[1:], "v:w:")
-    pdfviewer = ""
-    psviewer = ""
-    for o, v in opts:
-      if o == "-v":
-        pdfviewer = v
-      if o == "-w":
-        psviewer = v
+    args = sys.argv[1:]
     
     if len(args) < 1:
       error(usage(progname))
 
     result = find(args, path = os.environ["HOME"])
-    if result == "":
-        message("no document found!")
-        exit(2)
-    else:
-        message("found document %s" % result)
-
-    viewer = ""
-    if result.lower().endswith('.ps'):
-        if psviewer == "":
-            viewer = find_exe_or_terminate(ps_viewers)
-        else:
-            viewer = psviewer
-    else:
-        if pdfviewer == "":
-           viewer = find_exe_or_terminate(pdf_viewers)
-        else:
-            viewer = pdfviewer
-    
-    cmdline = viewer.split(" -", 1)
-
-    if len(cmdline) == 1:
-        subprocess.Popen([viewer, result], stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL)
-    elif len(cmdline) == 2:
-        subprocess.Popen([cmdline[0], "-" + cmdline[1] , result], stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL)
-    
+     
+    print(result)
     exit(0)
 
 if __name__ == "__main__":

src/frontends/qt/GuiView.cpp

@@ -5072,12 +5072,7 @@ void GuiView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
 
 		case LFUN_CITATION_OPEN: {
 			LASSERT(doc_buffer, break);
-			string pdfv, psv;
-			if (theFormats().getFormat("pdf"))
-				pdfv = theFormats().getFormat("pdf")->viewer();
-			if (theFormats().getFormat("ps"))
-				psv = theFormats().getFormat("ps")->viewer();
-			frontend::showTarget(argument, doc_buffer->absFileName(), pdfv, psv);
+			frontend::showTarget(argument, *doc_buffer);
 			break;
 		}
 

src/frontends/qt/qt_helpers.cpp

@@ -14,6 +14,7 @@
 
 #include "qt_helpers.h"
 
+#include "Format.h"
 #include "LengthCombo.h"
 #include "LyXRC.h"
 
@@ -32,6 +33,7 @@
 #include <QComboBox>
 #include <QDesktopServices>
 #include <QDir>
+#include <QInputDialog>
 #include <QLineEdit>
 #include <QMessageBox>
 #include <QLocale>
@@ -295,30 +297,61 @@ void showDirectory(FileName const & directory)
 				qstring_to_ucs4(qurl.toString())));
 }
 
-void showTarget(string const & target, string const & docpath,
-		string const & pdfv, string const & psv)
+void showTarget(string const & target_in, Buffer const & buf)
 {
-	LYXERR(Debug::INSETS, "Showtarget:" << target << "\n");
+	LYXERR(Debug::INSETS, "Showtarget:" << target_in << "\n");
 
+	string target = target_in;
+	string const & docpath = buf.absFileName();
+
+	bool const is_external = prefixIs(target, "EXTERNAL ");
+	if (is_external) {
+		if (!lyxrc.citation_search)
+			return;
+		string tmp, tar;
+		tar = split(target, tmp, ' ');
+		string const scriptcmd = subst(lyxrc.citation_search_view, "$${python}", os::python());
+		string const command = scriptcmd + " " + tar;
+		cmd_ret const ret = runCommand(commandPrep(command));
+		if (!ret.valid) {
+			// Script failed
+			frontend::Alert::error(_("Could not open file"),
+				_("The lyxpaperview script failed."));
+			return;
+		}
+		// lyxpaperview returns a \n-separated list of paths
+		vector<string> targets = getVectorFromString(rtrim(ret.result, "\n"), "\n");
+		if (targets.empty()) {
+			frontend::Alert::error(_("Could not open file"),
+				bformat(_("No file was found using the pattern `%1$s'."),
+					from_utf8(tar)));
+			return;
+		}
+		if (targets.size() > 1) {
+			QStringList files;
+			for (auto const & t : targets)
+				files << toqstr(t);
+			bool ok;
+			QString file = QInputDialog::getItem(nullptr, qt_("Multiple files found!"),
+							     qt_("Select the file that should be opened:"),
+							     files, 0, false, &ok);
+			if (!ok || file.isEmpty())
+				return;
+			target = fromqstr(file);
+		} else
+			target = targets.front();
+	}
 	// security measure: ask user before opening if document is not marked trusted.
 	QSettings settings;
 	if (!settings.value("trusted documents/" + toqstr(docpath), false).toBool()) {
 		QCheckBox * dontShowAgainCB = new QCheckBox();
 		dontShowAgainCB->setText(qt_("&Trust this document and do not ask me again!"));
 		dontShowAgainCB->setToolTip(qt_("If you check this, LyX will open all targets without asking for the given document in the future."));
-		docstring const warn =
-			prefixIs(target, "EXTERNAL ") ?
-					bformat(_("LyX will search your directory for files with the following keywords in their name "
-						  "and then open it in an external application, if a file is found:\n"
-						  "'%1$s'\n"
-						  "Be aware that this might entail security infringements!\n"
-						  "Only do this if you trust origin of the document and the keywords used!\n"
-						  "How do you want to proceed?"), from_utf8(target).substr(9, docstring::npos))
-				      : bformat(_("LyX wants to open the following link in an external application:\n"
-						  "%1$s\n"
-						  "Be aware that this might entail security infringements!\n"
-						  "Only do this if you trust origin of the document and the target of the link!\n"
-						  "How do you want to proceed?"), from_utf8(target));
+		docstring const warn = bformat(_("LyX wants to open the following target in an external application:\n"
+						 "%1$s\n"
+						 "Be aware that this might entail security infringements!\n"
+						 "Only do this if you trust origin of the document and the target of the link!\n"
+						 "How do you want to proceed?"), from_utf8(target));
 		QMessageBox box(QMessageBox::Warning, qt_("Open external target?"), toqstr(warn),
 				QMessageBox::NoButton, qApp->focusWidget());
 		QPushButton * openButton = box.addButton(qt_("&Open Target"), QMessageBox::ActionRole);
@@ -332,33 +365,23 @@ void showTarget(string const & target, string const & docpath,
 			settings.setValue("trusted documents/"
 				+ toqstr(docpath), true);
 	}
-	
-	if (prefixIs(target, "EXTERNAL ")) {
-		if (!lyxrc.citation_search)
-			return;
-		string tmp, tar, opts;
-		tar = split(target, tmp, ' ');
-		if (!pdfv.empty())
-			opts = " -v \"" + pdfv + "\"";
-		if (!psv.empty())
-			opts += " -w \"" + psv + "\"";
-		if (!opts.empty())
-			opts += " ";
-		Systemcall one;
-		string const viewer = subst(lyxrc.citation_search_view, "$${python}", os::python());
-		string const command = viewer + " " + opts + tar;
-		int const result = one.startscript(Systemcall::Wait, command);
-		if (result == 1)
-			// Script failed
-			frontend::Alert::error(_("Could not open file"),
-				_("The lyxpaperview script failed."));
-		else if (result == 2)
-			frontend::Alert::error(_("Could not open file"),
-				bformat(_("No file was found using the pattern `%1$s'."),
-					from_utf8(tar)));
-		return;
-	}
-	if (!QDesktopServices::openUrl(QUrl(toqstr(target), QUrl::TolerantMode)))
+
+	bool success = false;
+	QUrl url = is_external
+		? QUrl::fromLocalFile(toqstr(target))
+		: QUrl(toqstr(target), QUrl::TolerantMode);
+	if (url.isLocalFile()) {
+		// For local files, we use our own viewers
+		// (QDesktopServices employs xdg-open which
+		// does not yet work everywhere)
+		FileName fn(fromqstr(url.path()));
+		string const format = theFormats().getFormatFromFile(fn);
+		success = theFormats().view(buf, fn, format);
+	} else
+		// For external files, we rely on QDesktopServices
+		success =  QDesktopServices::openUrl(url);
+
+	if (!success)
 		frontend::Alert::error(_("Could not open file"),
 			bformat(_("The target `%1$s' could not be resolved."),
 				from_utf8(target)));

src/frontends/qt/qt_helpers.h

@@ -13,6 +13,7 @@
 #ifndef QTHELPERS_H
 #define QTHELPERS_H
 
+#include "Buffer.h"
 #include "ColorSet.h"
 #include "support/Length.h"
 #include "support/qstring_helpers.h"
@@ -93,12 +94,9 @@ void setMessageColour(std::list<QWidget *> highlighted,
 void showDirectory(support::FileName const & directory);
 /// handle request for showing citation content - shows pdf/ps or
 /// web page in target; external script can be used for pdf/ps view
-/// \p docpath holds the document path,
-/// \p pdfv takes a pad viewer, \p psv a ps viewer
+/// \p docpath holds the document path
 void showTarget(std::string const & target,
-		std::string const & docpath,
-		std::string const & pdfv,
-		std::string const & psv);
+		Buffer const & buf);
 
 } // namespace frontend