[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: PATCH: KonqRun::askSave and URLs containing '%'
From:       Maarten ter Huurne <mth () stack ! nl>
Date:       2001-08-29 20:52:20
[Download RAW message or body]

On Wednesday 29 August 2001 19:20, David Faure wrote:

> Looks good to me, except for the "if the arg is null then %n remains"
> specification. Shouldn't null give an empty substitution ? Or "(null)"
> maybe. But a %n in a message to be read by the user is going to look very
> ugly.

It is my intention that it will look ugly to the user, because null strings 
are probably bugs. If they are easy to spot, there is a better chance they 
get fixed.

Because the default arguments are null when not specified, it seemed like a 
logical choice to keep the "%n". So if you specify 3 arguments, "%4" and 
above will not be substituted and will remind you that you passed too little 
arguments.
Also, it was easy to implement, because "digit out of range" and "not a 
digit" can be handled in the same way.

> Note: your switch() lacks a "default:" case.

Is that dangerous or just unconventional?

From the patch:
+        const QString *argn = &QString::null;
+        switch(s[i].digitValue()) { // -1 if no digit
+            case 1: argn = &arg1; break;
+            case 2: argn = &arg2; break;
+            case 3: argn = &arg3; break;
+            case 4: argn = &arg4; break;
+            case 5: argn = &arg5; break;
+            case 6: argn = &arg6; break;
+            case 7: argn = &arg7; break;
+            case 8: argn = &arg8; break;
+            case 9: argn = &arg9; break;
+        }
+        if (!argn->isNull()) {
+            ret += *argn;
+            i++;
+        }
+        else ret += '%';

If none of the cases match, argn should still be pointing to QString::null 
which it was initialised to, causing no substitution to take place and the 
'%' being added to the output instead.

> Note2: you don't have to use pointers with QString. QString a = b is very
> cheap (it's a shared class).

OK, that's good to know.

Bye,
		Maarten

[prev in list] [next in list] [prev in thread] [next in thread]