View Issue Details

IDProjectCategoryView StatusLast Update
0013413ScribusUser Interfacepublic2015-11-03 01:03
ReporterjurajF Assigned Tocbradney  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformqemuOSLinuxOS VersionDebian 8/Jessie
Product Version1.5.0 
Fixed in Version1.5.1svn 
Summary0013413: [CLI] Error-message-wrong-and-too-verbose
DescriptionWhile fixing 0013408 I fixed some more issues in function
void ScribusQApp::parseCommandLine()

0002-Error-message-wrong-and-too-verbose.patch

When I run scribus without provided script I get wrong/confusing error
message that is buried in between of boilerplate text (hard to read):

$ scribus-1.5.1.svn -py -l ru

Scribus, Open Source Desktop Publishing
---------------------------------------
Homepage:       http://www.scribus.net
Documentation:  http://docs.scribus.net
Wiki:           http://wiki.scribus.net
Issues:         http://bugs.scribus.net

Invalid argument: -l
Usage: scribus [options] [files]

Options:
     -fi,  --font-info                      Show information on the console when fonts are being loaded
     -h,   --help                           Print help (this message) and exit
     -l,   --lang                           Uses xx as shortcut for a language, eg `en' or `de'
     -la,  --langs-available                List the currently installed interface languages
     -ns,  --no-splash                      Do not show the splashscreen on startup
     -nns, --never-splash                   Stop showing the splashscreen on startup. Writes an empty file called .neversplash in ~/.scribus
     -pr,  --prefs <filename>               Use filename as path for user given preferences
     -pi,  --profile-info                   Show location of ICC profile information on console while starting
     -u,   --upgradecheck                   Download a file from the Scribus website and show the latest available version
     -v,   --version                        Output version information and exit
     -py,  --python-script <filename>       Run filename in Python scripter
     -pa,  --python-arg <argument> [value]  Argument passed on to python script, with an optional value, no effect without -py
     -g,   --no-gui                         Do not start GUI
     --                                     Explicit end of command line options

Scribus Version 1.5.1.svn

It is not that -l argument is invalid but -py argument is missing filename.

---
Running like this produce confusing message (I will omit boilerplate text
from error message here):
$ scribus-1.5.1.svn document.sla -g
...
Invalid argument: -g
...


Here I would expect scribus to behave exactly the same as if run like:
$ scribus-1.5.1.svn -g document.sla
that is: open document.sla file and then quit immediately. But I did not
fix it to act like this. Instead I followed the output of:
$ scribus-1.5.1.svn -h
...
Usage: scribus [options] [files]
...

Which means that all files that scribus will open must be placed after
all argument. In this case -g would be interpreted as filename to be
opened - which will most probably produce error message if you do not
have file named -g (and you do have document.sla!):

$ scribus-1.5.1.svn document.sla -g
...
File -g does not exist, aborting.
...

Steps To Reproduce$ scribus-1.5.1.svn -py -l ru
$ scribus-1.5.1.svn document.sla -g
Additional InformationCloned from 0013408
TagsNo tags attached.
PatchYes

Relationships

related to 0013408 closedcbradney [CLI] --prefs option without provided file causes crash 
related to 0013415 closedcbradney [CLI] Changing multiple flag rules 
related to 0013414 closedcbradney [CLI] --lang issues 
related to 0013452 closedfschmid Can not open ".autosave" file: "is not in an acceptable format" 

Activities

2015-10-10 13:43

 

0002-Error-message-wrong-and-too-verbose.patch (2,977 bytes)   
From 81722cee37d8d8ff6e7862ab738661c174fbc2ec Mon Sep 17 00:00:00 2001
From: Juraj Fedel <wtxnh-scribus@yahoo.com.au>
Date: Sat, 10 Oct 2015 12:25:07 +0200
Subject: [PATCH 2/7] Error message wrong and too verbose

---
 scribus/scribusapp.cpp | 22 +++++++---------------
 scribus/scribusapp.h   |  1 -
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/scribus/scribusapp.cpp b/scribus/scribusapp.cpp
index f9b0eec..233eed5 100644
--- a/scribus/scribusapp.cpp
+++ b/scribus/scribusapp.cpp
@@ -235,7 +235,7 @@ void ScribusQApp::parseCommandLine()
 			}
 			prefsUserFile = QFile::decodeName(args[argi + 1].toLocal8Bit());
 			if (!QFileInfo(prefsUserFile).exists()) {
-				showError(prefsUserFile);
+				std::cout << tr("Preferenes file %1 does not exist, aborting.").arg(prefsUserFile).toLocal8Bit().data() << std::endl;
 				std::exit(EXIT_FAILURE);
 			} else {
 				++argi;
@@ -250,7 +250,7 @@ void ScribusQApp::parseCommandLine()
 			}
 			pythonScript = QFile::decodeName(args[argi + 1].toLocal8Bit());
 			if (!QFileInfo(pythonScript).exists()) {
-				showError(pythonScript);
+				std::cout << tr("Python script %1 does not exist, aborting.").arg(pythonScript).toLocal8Bit().data() << std::endl;
 				std::exit(EXIT_FAILURE);
 			} else {
 				++argi;
@@ -259,6 +259,10 @@ void ScribusQApp::parseCommandLine()
 			argi++;
 			break;
 		} else { //argument is not a known option, but either positional parameter or invalid.
+			if (arg.left(1) == "-") {
+				std::cout << tr("Invalid argument: %1").arg(arg).toLocal8Bit().data() << std::endl;
+				std::exit(EXIT_FAILURE);
+			}
 			break;
 		}
 	}
@@ -266,7 +270,7 @@ void ScribusQApp::parseCommandLine()
 	for ( ; argi<argsc; argi++) {
 		fileName = QFile::decodeName(args[argi].toLocal8Bit());
 		if (!QFileInfo(fileName).exists()) {
-			showError(fileName);
+			std::cout << tr("File %1 does not exist, aborting.").arg(fileName).toLocal8Bit().data() << std::endl;
 			std::exit(EXIT_FAILURE);
 		} else {
 			filesToLoad.append(fileName);
@@ -591,18 +595,6 @@ void ScribusQApp::showHeader()
 	endl(ts);
 }
 
-void ScribusQApp::showError(QString arg)
-{
-	showHeader();
-	if (arg.left(1) == "-" || arg.left(2) == "--") {
-		std::cout << tr("Invalid argument: %1").arg(arg).toLocal8Bit().data() << std::endl;
-	} else {
-		std::cout << tr("File %1 does not exist, aborting.").arg(arg).toLocal8Bit().data() << std::endl;
-	}
-	showUsage();
-	std::cout << tr("Scribus Version").toLocal8Bit().data() << " " << VERSION << std::endl;
-}
-
 void ScribusQApp::neverSplash(bool splashOff)
 {
 	QString prefsDir = ScPaths::getApplicationDataDir();
diff --git a/scribus/scribusapp.h b/scribus/scribusapp.h
index 0add48f..7fc9f7f 100644
--- a/scribus/scribusapp.h
+++ b/scribus/scribusapp.h
@@ -77,7 +77,6 @@ class SCRIBUS_API ScribusQApp : public QApplication
 	private:
 		ScribusCore* m_ScCore;
 		void showHeader();
-		void showError(QString arg);
 		void showVersion();
 		/*!
 		\author Franz Schmid
-- 
2.1.4

Kunda

2015-10-11 22:50

updater   ~0036632

Coolness. Thanks Juraj. We should get Berteh in to this conversation as well. I'll try to ping him.

berteh

2015-10-11 22:50

reporter   ~0036633

Hi Juraj, thanks for the various improvements you brought esp. improving the feedback when arguments are missing a required value (filename in these cases).

just one note regarding your use of "--". I think it's wrong to allow multiple use of it. It usually (as in "in other places where it's used) denotes the end of options and the beginning of positional arguments... and I think it should stay that way.

The reason you mention for needing to enable multiple use was actually not needed, as -pa works just fine if you provide it with just a flag name and then follow with more options (it will detect the starting "-" of the following option and skip looking for its (optional) argument value)... so the only place you needed the "--" is indeed at the very end of options sequence, to make sure the positional arguments (scribus files) are not consumed by -pa.

[it was handled in the line
if (++argi < argsc && !args[argi].startsWith("-")) { // arg value
that you removed in your patch "strange use of delimiter") ]



That being said, I agree with you that forcing -pa to be the last option and consume all that's left of command line (before an optional --) would be less verbose. It's just that othes preferred not having that mechanism, see aoloe at https://github.com/scribusproject/scribus/pull/19#issuecomment-128313027, and cbradney at http://bugs.scribus.net/view.php?id=13311#c36165

... so I guess that part is not an option for the core devs of scribus.

Kunda

2015-10-11 22:52

updater   ~0036634

Last edited: 2015-10-11 23:47

Cloned this issue 0013408 to split up the ticket so we can talk about the different patches in ea. ticket.

Kunda

2015-10-20 23:27

updater   ~0036756

Scribus CLI enthusiasts...please continue the conversation about this patch.

Kunda

2015-11-03 01:03

updater   ~0037160

Committed in r20513 by Craig
Patched by JurajF
Thanks

Issue History

Date Modified Username Field Change
2015-10-11 22:50 Kunda New Issue
2015-10-11 22:50 Kunda Issue generated from: 0013408
2015-10-11 22:50 Kunda Relationship added related to 0013408
2015-10-11 22:51 Kunda File Deleted: 0007-Allow-sla-files-to-be-intermixed-with-other-options.patch
2015-10-11 22:51 Kunda File Deleted: 0006-Merge-ARG_SCRIPTARG-with-ARG_PYTHONSCRIPT.patch
2015-10-11 22:51 Kunda File Deleted: 0005-Strange-use-of-delimiter.patch
2015-10-11 22:51 Kunda File Deleted: 0004-Wrong-error-when-lang-does-not-have-an-argument.patch
2015-10-11 22:51 Kunda File Deleted: 0003-Remove-duplicated-code.patch
2015-10-11 22:52 Kunda File Deleted: 0001-prefs-option-without-provided-file-causes-crash.patch
2015-10-11 22:52 Kunda Note Added: 0036634
2015-10-11 22:57 Kunda Issue cloned: 0013414
2015-10-11 22:57 Kunda Relationship added related to 0013414
2015-10-11 23:45 Kunda Reporter Kunda => jurajF
2015-10-11 23:45 Kunda Relationship added related to 0013415
2015-10-11 23:47 Kunda Note Edited: 0036634
2015-10-20 23:27 Kunda Note Added: 0036756
2015-10-29 04:44 Kunda Relationship added related to 0013452
2015-11-02 09:59 cbradney Status new => resolved
2015-11-02 09:59 cbradney Fixed in Version => 1.5.1svn
2015-11-02 09:59 cbradney Resolution open => fixed
2015-11-02 09:59 cbradney Assigned To => cbradney
2015-11-03 01:03 Kunda Note Added: 0037160
2015-11-03 01:03 Kunda Status resolved => closed