[Libreoffice-commits] core.git: desktop/qa desktop/source

Mike Kaganski mike.kaganski at collabora.com
Sun Jul 31 09:35:33 UTC 2016


 desktop/qa/desktop_app/test_desktop_app.cxx |   56 +++++++---
 desktop/source/app/cmdlineargs.cxx          |  147 ++++++++++++----------------
 2 files changed, 105 insertions(+), 98 deletions(-)

New commits:
commit 4311e8fb88c334cccad6f577610e1af8ae75bc59
Author: Mike Kaganski <mike.kaganski at collabora.com>
Date:   Thu Jul 28 21:41:11 2016 +1000

    tdf#100837: honor LibreOffice command line arguments for Office URIs
    
    This patch allows modifying open mode set in Office URI by using
    LibreOffice usual command line arguments. For instance, if there is
    -p "ofe:...", then the file won't be open for editing, but instead
    will be printed.
    
    Change-Id: I4bde9b6e1c0e92b63ee3834ee1fd8f6e1bd321f2
    Reviewed-on: https://gerrit.libreoffice.org/27629
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>

diff --git a/desktop/qa/desktop_app/test_desktop_app.cxx b/desktop/qa/desktop_app/test_desktop_app.cxx
index c800b2c..9d8225e 100644
--- a/desktop/qa/desktop_app/test_desktop_app.cxx
+++ b/desktop/qa/desktop_app/test_desktop_app.cxx
@@ -47,6 +47,7 @@ public:
 
 class TestSupplier : public desktop::CommandLineArgs::Supplier {
 public:
+    TestSupplier(std::initializer_list<OUString> args) : m_args(args) {}
     virtual ~TestSupplier() {}
     virtual boost::optional< OUString > getCwdUrl() override { return boost::optional< OUString >(); }
     virtual bool next(OUString * argument) override {
@@ -59,7 +60,6 @@ public:
             return false;
         }
     }
-    TestSupplier& operator << (const OUString& arg) { m_args.push_back(arg); return *this; }
 private:
     std::vector< OUString > m_args;
     std::vector< OUString >::size_type m_index = 0;
@@ -73,23 +73,43 @@ void Test::testTdf100837() {
     // Without this we're crashing because callees are using getProcessServiceFactory
     ::comphelper::setProcessServiceFactory(xSM);
 
-    TestSupplier supplier;
-    supplier << "--view" << "foo" << "ms-word:ofe|u|bar1" << "ms-word:ofv|u|bar2" << "ms-word:nft|u|bar3" << "baz";
-    desktop::CommandLineArgs args(supplier);
-    auto vViewList = args.GetViewList();
-    auto vForceOpenList = args.GetForceOpenList();
-    auto vForceNewList = args.GetForceNewList();
-    // 3 documents go to View list: foo; bar2; baz
-    CPPUNIT_ASSERT_EQUAL(decltype(vViewList.size())(3), vViewList.size());
-    CPPUNIT_ASSERT_EQUAL(OUString("foo"),  vViewList[0]);
-    CPPUNIT_ASSERT_EQUAL(OUString("bar2"), vViewList[1]);
-    CPPUNIT_ASSERT_EQUAL(OUString("baz"),  vViewList[2]);
-    // 1 document goes to ForceOpen list: bar1
-    CPPUNIT_ASSERT_EQUAL(decltype(vForceOpenList.size())(1), vForceOpenList.size());
-    CPPUNIT_ASSERT_EQUAL(OUString("bar1"), vForceOpenList[0]);
-    // 1 document goes to ForceNew list: bar3
-    CPPUNIT_ASSERT_EQUAL(decltype(vForceNewList.size())(1), vForceNewList.size());
-    CPPUNIT_ASSERT_EQUAL(OUString("bar3"), vForceNewList[0]);
+    {
+        // 1. Test default behaviour: Office URIs define open mode
+        TestSupplier supplier{ "foo", "ms-word:ofe|u|bar1", "ms-word:ofv|u|bar2", "ms-word:nft|u|bar3", "baz" };
+        desktop::CommandLineArgs args(supplier);
+        auto vOpenList      = args.GetOpenList();
+        auto vForceOpenList = args.GetForceOpenList();
+        auto vViewList      = args.GetViewList();
+        auto vForceNewList  = args.GetForceNewList();
+        // 2 documents go to Open list: foo; baz
+        CPPUNIT_ASSERT_EQUAL(decltype(vOpenList.size())(2), vOpenList.size());
+        CPPUNIT_ASSERT_EQUAL(OUString("foo"), vOpenList[0]);
+        CPPUNIT_ASSERT_EQUAL(OUString("baz"), vOpenList[1]);
+        // 1 document goes to ForceOpen list: bar1
+        CPPUNIT_ASSERT_EQUAL(decltype(vForceOpenList.size())(1), vForceOpenList.size());
+        CPPUNIT_ASSERT_EQUAL(OUString("bar1"), vForceOpenList[0]);
+        // 1 document goes to View list: bar2
+        CPPUNIT_ASSERT_EQUAL(decltype(vViewList.size())(1), vViewList.size());
+        CPPUNIT_ASSERT_EQUAL(OUString("bar2"), vViewList[0]);
+        // 1 document goes to ForceNew list: bar3
+        CPPUNIT_ASSERT_EQUAL(decltype(vForceNewList.size())(1), vForceNewList.size());
+        CPPUNIT_ASSERT_EQUAL(OUString("bar3"), vForceNewList[0]);
+    }
+
+    {
+        // 2. Test explicit open mode arguments. Office URI commands should have no effect
+        TestSupplier supplier{ "--view", "ms-word:ofe|u|foo", "-o", "ms-word:ofv|u|bar", "ms-word:nft|u|baz" };
+        desktop::CommandLineArgs args(supplier);
+        auto vViewList      = args.GetViewList();
+        auto vForceOpenList = args.GetForceOpenList();
+        // 1 document goes to View list: foo
+        CPPUNIT_ASSERT_EQUAL(decltype(vViewList.size())(1), vViewList.size());
+        CPPUNIT_ASSERT_EQUAL(OUString("foo"), vViewList[0]);
+        // 2 documents go to ForceOpen list: bar, baz
+        CPPUNIT_ASSERT_EQUAL(decltype(vForceOpenList.size())(2), vForceOpenList.size());
+        CPPUNIT_ASSERT_EQUAL(OUString("bar"),  vForceOpenList[0]);
+        CPPUNIT_ASSERT_EQUAL(OUString("baz"), vForceOpenList[1]);
+    }
 }
 
 CPPUNIT_TEST_SUITE_REGISTRATION(Test);
diff --git a/desktop/source/app/cmdlineargs.cxx b/desktop/source/app/cmdlineargs.cxx
index f13e01c..f9a2758 100644
--- a/desktop/source/app/cmdlineargs.cxx
+++ b/desktop/source/app/cmdlineargs.cxx
@@ -102,82 +102,74 @@ private:
     sal_uInt32 m_index;
 };
 
-// Office URI Schemes : see https://msdn.microsoft.com/en-us/library/dn906146
-class OfficeURISchemeCommandLineSupplier : public CommandLineArgs::Supplier {
-public:
-    static bool IsOfficeURI(const OUString& URI, OUString* rest = nullptr)
+enum class CommandLineEvent {
+    Open, Print, View, Start, PrintTo,
+    ForceOpen, ForceNew, Conversion, BatchPrint
+};
+
+// Office URI Schemes: see https://msdn.microsoft.com/en-us/library/dn906146
+// This functions checks if the arg is an Office URI.
+// If applicable, it updates arg to inner URI.
+// If no event argument is explicitly set in command line,
+// then it returns updated command line event,
+// according to Office URI command.
+CommandLineEvent CheckOfficeURI(/* in,out */ OUString& arg, CommandLineEvent curEvt)
+{
+    // 1. Strip the scheme name
+    OUString rest1;
+    bool isOfficeURI = ( arg.startsWithIgnoreAsciiCase("vnd.libreoffice.command:", &rest1) // Proposed extended schema
+                      || arg.startsWithIgnoreAsciiCase("ms-word:",                 &rest1)
+                      || arg.startsWithIgnoreAsciiCase("ms-powerpoint:",           &rest1)
+                      || arg.startsWithIgnoreAsciiCase("ms-excel:",                &rest1)
+                      || arg.startsWithIgnoreAsciiCase("ms-visio:",                &rest1)
+                      || arg.startsWithIgnoreAsciiCase("ms-access:",               &rest1));
+    if (!isOfficeURI)
+        return curEvt;
+
+    OUString rest2;
+    long nURIlen = -1;
+    // 2. Discriminate by command name (incl. 1st command argument descriptor)
+    //    Extract URI: everything up to possible next argument
+    if (rest1.startsWith("ofv|u|", &rest2))
     {
-        return ( URI.startsWithIgnoreAsciiCase("vnd.libreoffice.command:", rest) // Proposed extended schema
-              || URI.startsWithIgnoreAsciiCase("ms-word:",                 rest)
-              || URI.startsWithIgnoreAsciiCase("ms-powerpoint:",           rest)
-              || URI.startsWithIgnoreAsciiCase("ms-excel:",                rest)
-              || URI.startsWithIgnoreAsciiCase("ms-visio:",                rest)
-              || URI.startsWithIgnoreAsciiCase("ms-access:",               rest));
+        // Open for view - override only in default mode
+        if (curEvt == CommandLineEvent::Open)
+            curEvt = CommandLineEvent::View;
+        nURIlen = rest2.indexOf("|");
     }
-    OfficeURISchemeCommandLineSupplier(boost::optional< OUString > cwdUrl, const OUString& URI)
-        : m_cwdUrl(cwdUrl)
+    else if (rest1.startsWith("ofe|u|", &rest2))
     {
-        // 1. Strip the scheme name
-        OUString rest1;
-        bool isOfficeURI = IsOfficeURI(URI, &rest1);
-        assert(isOfficeURI);
-        (void) isOfficeURI;
-
-        OUString rest2;
-        long nURIlen = -1;
-        // 2. Discriminate by command name (incl. 1st command argument descriptor)
-        //    Extract URI: everything up to possible next argument
-        if (rest1.startsWith("ofv|u|", &rest2))
-        {
-            // Open for view
-            m_args.push_back("--view");
-            nURIlen = rest2.indexOf("|");
-        }
-        else if (rest1.startsWith("ofe|u|", &rest2))
-        {
-            // Open for editing
-            m_args.push_back("-o");
-            nURIlen = rest2.indexOf("|");
-        }
-        else if (rest1.startsWith("nft|u|", &rest2))
-        {
-            // New from template
-            m_args.push_back("-n");
-            nURIlen = rest2.indexOf("|");
-            // TODO: process optional second argument (default save-to location)
-            // For now, we just ignore it
-        }
-        else
-        {
-            // Abbreviated schema: <scheme-name>:URI
-            // "ofv|u|" implied
-            rest2 = rest1;
-            m_args.push_back("--view");
-        }
-        if (nURIlen < 0)
-            nURIlen = rest2.getLength();
-        m_args.push_back(rest2.copy(0, nURIlen));
+        // Open for editing - override only in default mode
+        if (curEvt == CommandLineEvent::Open)
+            curEvt = CommandLineEvent::ForceOpen;
+        nURIlen = rest2.indexOf("|");
     }
-    virtual ~OfficeURISchemeCommandLineSupplier() {}
-    virtual boost::optional< OUString > getCwdUrl() override { return m_cwdUrl; }
-    virtual bool next(OUString * argument) override {
-        assert(argument != nullptr);
-        if (m_index < m_args.size()) {
-            *argument = m_args[m_index++];
-            return true;
-        }
-        else {
-            return false;
-        }
+    else if (rest1.startsWith("nft|u|", &rest2))
+    {
+        // New from template - override only in default mode
+        if (curEvt == CommandLineEvent::Open)
+            curEvt = CommandLineEvent::ForceNew;
+        nURIlen = rest2.indexOf("|");
+        // TODO: process optional second argument (default save-to location)
+        // For now, we just ignore it
     }
-private:
-    boost::optional< OUString > m_cwdUrl;
-    std::vector< OUString > m_args;
-    std::vector< OUString >::size_type m_index = 0;
-};
-
+    else
+    {
+        // Abbreviated scheme: <scheme-name>:URI
+        // "ofv|u|" implied
+        // override only in default mode
+        if (curEvt == CommandLineEvent::Open)
+            curEvt = CommandLineEvent::View;
+        rest2 = rest1;
+    }
+    if (nURIlen < 0)
+        nURIlen = rest2.getLength();
+    arg = rest2.copy(0, nURIlen);
+    return curEvt;
 }
 
+} // namespace
+
 CommandLineArgs::Supplier::Exception::Exception() {}
 
 CommandLineArgs::Supplier::Exception::Exception(Exception const &) {}
@@ -207,9 +199,6 @@ CommandLineArgs::CommandLineArgs( Supplier& supplier )
 void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
 {
     m_cwdUrl = supplier.getCwdUrl();
-
-    enum class CommandLineEvent { Open, Print, View, Start, PrintTo,
-                                  ForceOpen, ForceNew, Conversion, BatchPrint };
     CommandLineEvent eCurrentEvent = CommandLineEvent::Open;
 
     for (;;)
@@ -534,14 +523,12 @@ void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
             {
                 // handle this argument as a filename
 
-                // 1. Check if this is an Office URI
-                if (OfficeURISchemeCommandLineSupplier::IsOfficeURI(aArg))
-                {
-                    OfficeURISchemeCommandLineSupplier OfficeURISupplier(getCwdUrl(), aArg);
-                    // Add the file according its command, ignore current event
-                    ParseCommandLine_Impl(OfficeURISupplier);
-                }
-                else switch (eCurrentEvent)
+                // First check if this is an Office URI
+                // This will possibly adjust event for this argument
+                // and put real URI to aArg
+                CommandLineEvent eThisEvent = CheckOfficeURI(aArg, eCurrentEvent);
+
+                switch (eThisEvent)
                 {
                 case CommandLineEvent::Open:
                     m_openlist.push_back(aArg);


More information about the Libreoffice-commits mailing list