Icons for: Tab functions

I have committed the icons and the minor change to use the
new icon: 
"tab_close_other" to the 3.5 BRANCH.

Therefore, I attach revised patches for the 'open in new
tab' trick.

-- 
JRT

On Wednesday 05 April 2006 07:51, James Richard Tyrer wrote:
> +    KConfig *config = KGlobal::config();
> +    config->setGroup( "FMSettings" );
> +    bool newtabsinfront = config->readBoolEntry(
"NewTabsInFront", false );

Please use KonqSettings::newTabsInFront() instead.

-- 
David Faure, faurekde.org, sponsored by Trolltech to work on
KDE,
Konqueror (http://www.konqueror.org
), and KOffice (http://www.koffice.org).

On Wednesday 05 April 2006 22:41, David Faure wrote:
> On Wednesday 05 April 2006 07:51, James Richard Tyrer
wrote:
> > +    KConfig *config = KGlobal::config();
> > +    config->setGroup( "FMSettings"
);
> > +    bool newtabsinfront =
config->readBoolEntry( "NewTabsInFront",
false );
> 
> Please use KonqSettings::newTabsInFront() instead.
... in kdebase only; not available in kdelibs.

In kdelibs it's a bit strange to access konqueror's
settings from khtml.
In theory khtml could be used by other browser or
pseudo-browser implementations...
I guess khtml would need a boolean property for this, or
more generally a newTabIconName QString property;
konqueror could then set that property [using dynamic code
since it doesn't link to khtml,
but we have other examples of that already]

PS: no static QStrings please, they take memory and might
give trouble in some environments (just remove the static)

-- 
David Faure, faurekde.org, sponsored by Trolltech to work on
KDE,
Konqueror (http://www.konqueror.org
), and KOffice (http://www.koffice.org).

On Thursday 06 April 2006 00:10, David Faure wrote:
> On Wednesday 05 April 2006 22:41, David Faure wrote:
> > On Wednesday 05 April 2006 07:51, James Richard
Tyrer wrote:
> > > +    KConfig *config = KGlobal::config();
> > > +    config->setGroup(
"FMSettings" );
> > > +    bool newtabsinfront =
config->readBoolEntry( "NewTabsInFront",
false );
> > 
> > Please use KonqSettings::newTabsInFront() instead.
> ... in kdebase only; not available in kdelibs.
> 
> In kdelibs it's a bit strange to access konqueror's
settings from khtml.
> In theory khtml could be used by other browser or
pseudo-browser implementations...
> I guess khtml would need a boolean property for this,
or more generally a newTabIconName QString property;
> konqueror could then set that property [using dynamic
code since it doesn't link to khtml,
> but we have other examples of that already]

OK here's a better solution. Apply /k/browserextension.diff
to kdelibs/kparts, then you can use
khtml->browserExtension()->newTabsInFront() from
khtml_ext.cpp

Now all that still needs to be done is to set that bool from
konqueror....
Hmm, but there it makes req.newTabInFront redundant (where
req is a KonqOpenURLRequest),
since it would be equivalent to req.args.newTabsInFront()
now...
So I guess the right thing to do would be to change
konqueror to remove 
newTabInFront from konq_openurlrequest.h and use
req.args.newTabsInFront()/setNewTabsInFront() instead.
No time to do this today though, so if it's too much code
change for you I can do it another day,
otherwise feel free to go ahead with that change.

-- 
David Faure, faurekde.org, sponsored by Trolltech to work on
KDE,
Konqueror (http://www.konqueror.org
), and KOffice (http://www.koffice.org).

David Faure wrote:
> On Thursday 06 April 2006 00:10, David Faure wrote:
>> On Wednesday 05 April 2006 22:41, David Faure
wrote:
>>> On Wednesday 05 April 2006 07:51, James Richard
Tyrer wrote:
>>>> +    KConfig *config = KGlobal::config();
>>>> +    config->setGroup(
"FMSettings" );
>>>> +    bool newtabsinfront =
config->readBoolEntry( "NewTabsInFront",
false );
>>> Please use KonqSettings::newTabsInFront()
instead.
>> ... in kdebase only; not available in kdelibs.
>>
>> In kdelibs it's a bit strange to access
konqueror's settings from khtml.
>> In theory khtml could be used by other browser or
pseudo-browser implementations...
>> I guess khtml would need a boolean property for
this, or more generally a newTabIconName QString property;
>> konqueror could then set that property [using
dynamic code since it doesn't link to khtml,
>> but we have other examples of that already]
> 
> OK here's a better solution. Apply
/k/browserextension.diff to kdelibs/kparts, then you can use
> khtml->browserExtension()->newTabsInFront() from
khtml_ext.cpp
> 
> Now all that still needs to be done is to set that bool
from konqueror....
> Hmm, but there it makes req.newTabInFront redundant
(where req is a KonqOpenURLRequest),
> since it would be equivalent to
req.args.newTabsInFront() now...
> So I guess the right thing to do would be to change
konqueror to remove 
> newTabInFront from konq_openurlrequest.h and use
req.args.newTabsInFront()/setNewTabsInFront() instead.
> No time to do this today though, so if it's too much
code change for you I can do it another day,
> otherwise feel free to go ahead with that change.
> 

Thank you for your help with C++, Qt, KDE programing.  I
presume that 
eventually I will understand it.

This is more than my procedural language oriented brain can
easily 
digest.  So, I will have to study it a little.  So, if you
have time to 
do this first, please go ahead -- probably better than if I
did 
something which I didn't completely understand.

I made the other changes as you suggested and attach the
modified 
patches.  I am currently testing to see if they build OK and
will post 
if there is a problem.

I note that the question of whether to make a string in a
function 
"static" is a trade off.  I was taught to avoid
putting a string on the 
stack (which is where a regular C string in a function will
go if it 
isn't "static").  You do save memory by using
the stack, but at the 
expense of some processing speed -- which is often the trade
off. 
Without knowing exactly how QString works, I really
couldn't say what 
the optimum method is or whether declaring it:

	QString tab_new_x="         " // 9 blanks

would improve performance or not.  It would with a standard
C string 
(which would be 10 characters: 9 blanks and a
"\0"), since you would not 
need to move the string to resize it (which is very messy if
it is on 
the stack) if you allocated enough space for the longest
string that 
would be stored there.

-- 
JRT

James Richard Tyrer wrote:

> I made the other changes as you suggested and attach
the modified 
> patches.  I am currently testing to see if they build
OK and will post 
> if there is a problem.

No this isn't correct.  Having a bad day I guess.  But,
this is why I 
don't commit stuff till I have tested it.

So, I attach what I hope is the final patch for KDEBase.  I
will test 
this a little this evening and if all is well, I will commit
it to 3.5.2 
Branch.

-- 
JRT

On Friday 07 April 2006 01:55, James Richard Tyrer wrote:
> I note that the question of whether to make a string in
a function 
> "static" is a trade off.  I was taught to
avoid putting a string on the 
> stack (which is where a regular C string in a function
will go if it 
> isn't "static").  You do save memory by
using the stack, but at the 
> expense of some processing speed -- which is often the
trade off. 

Right. But KDE runs through a lot of methods, so if they all
kept their
data static, the memory consumption would be even bigger.
Speed in a user-triggered function doesn't matter -that-
much that a QString
constructor would be a penalty.

> Without knowing exactly how QString works, I really
couldn't say what 
> the optimum method is or whether declaring it:
> 
> 	QString tab_new_x="         " // 9 blanks
> 
> would improve performance or not.  It would with a
standard C string  
> (which would be 10 characters: 9 blanks and a
"\0"), since you would not 
> need to move the string to resize it (which is very
messy if it is on 
> the stack) if you allocated enough space for the
longest string that 
> would be stored there.

QString allocates data using new internally, it's not
really on the stack
(only the wrapper is on the stack).

Thanks for the konq commit, looks fine.
We still have to sort out khtml - but not today, for me.

-- 
David Faure, faurekde.org, sponsored by Trolltech to work on
KDE,
Konqueror (http://www.konqueror.org
), and KOffice (http://www.koffice.org).