Diego Queiroz | 1 May 01:53 2011
Picon

Python version?

Is there some consensus about what version of Python LyX scripts should be compatible?


I mean, may I propose some change in "configure.py", for example, that is only Python 2.6+ compatible or it should be compatible with previous versions?
If so, what's the version?


Regards,
---
Diego Queiroz

venom00 | 1 May 03:14 2011

RE: Memory LyXs

> I've looked briefly at this one before, and I find it confusing.  It
> looks as if it is saying that we are leaking the docstring 
> word itself,
> i.e., that it's not being cleaned up, but why would that be?

Mmmh, I find it pretty simple: we have some elements added to a set but never removed.

> Other question: Doesn't
> ==22649== 47,920 bytes in 960 blocks are indirectly lost in 
> loss record
> 920 of 921
> 
> say that there are just 47K bytes, which are scattered through 960
> blocks, that have been lost? That would make more sense to me, since
> they'd average about 50 bytes a piece, which looks about the size of a
> string. If so, the leak isn't that terrible.

Well, they're 47 kB for a short session, but in a session of several days/week... If I understand it
correctly, we leak every single word is inserted in a paragraph, as we never delete them.
When valgrind says that something is "indirectly lost" means that you have, for instance, a tree and you
deleted the root but not all the children [1]. It looks like our sitaution.

> Anyway, the obvious thing to do here is uncomment the code you
> identified and see if that helps.

I'll try that. Maybe we could add a bool parameter to WordList::remove to decide if really remove the node or
just ignore it. I'd ask to the author of the code, sts (from svn blame), is he still active?

> The other thing maybe worth trying is something simple like:
> 
> Index: src/Paragraph.cpp
> ===================================================================
> --- src/Paragraph.cpp (revision 38560)
> +++ src/Paragraph.cpp (working copy)
>  <at>  <at>  -3500,12 +3500,11  <at>  <at> 
>     pos_type from = pos;
>     locateWord(from, pos, WHOLE_WORD);
>     if (pos - from >= minlength) {
> -     docstring word = asString(from, pos, AS_STR_NONE);
>       FontList::const_iterator cit = d->fontlist_.fontIterator(pos);
>       if (cit == d->fontlist_.end())
> -       return;
> +       break;
>       Language const * lang = cit->font().language();
> -     d->words_[*lang].insert(word);
> +     d->words_[*lang].insert(asString(from, pos, AS_STR_NONE););
>     }
>   }
>  }

set<T>.insert() stores a copy [2] so you have to remove it manually. word should die when goes out of scope.

Thanks. I hope we can defeat this and other memory leaks and make LyX more lightweight.
venom00

[1] http://valgrind.org/docs/manual/mc-manual.html#mc-manual.errormsgs
[2] http://stackoverflow.com/questions/5122963/does-setinsert-saves-a-copy-or-a-pointer-c

Vincent van Ravesteijn | 1 May 13:10 2011

Re: Development for LyX 2.1

On 28-4-2011 19:23, Rob Oakes wrote:
>> I'm happy to go with git, as long as we're all going to get this kind of help.
> 
> It sounds as though consensus is emerging. Would it be possible
> to have the the git master branch mirrored into the existing SVN
> repository?  This would make it easy for those of us trying to
> maintain packages to have a "testing" branch.

What do you exactly mean with "trying to maintain packages" and
"to have a testing branch" ?

Vincent

Vincent van Ravesteijn | 1 May 13:29 2011

Re: Development for LyX 2.1

> 
> I'm also glad that my only objection with the above list is 
> c) develop new features in branches. I don't think it's necessary
> for most features, only for largish ones that are likely to take
> time and break the normal bug-free level in the main dev branch.
>

I understand that it sounds a bit over-the-top to create a new
feature for each commit you make. However let me point you to
the following observations I made:
- even the simplest commits, might have follow-ups, style
  corrections, correction of unwanted committed files,
  and so forth.

- it is good to have some safeguard for each commit. One might
  always forget some files, commit unwanted files, or make a
  stupid thinko. My problem is that each error in committing
  to svn leads to a cripple history.

- even if it might be unnecessary, it is hardly any extra
  effort. If the commit seems to be fine, all what is needed
  is:
  *  git merge feature
  *  git branch -d feature
> 
> One thing I would add to the list is
> 
> j) keep development alive during alpha-beta-rc stage for new
>    features that are not intended for this release but the next

Yes, I agree on this as well.

Vincent

Vincent van Ravesteijn | 1 May 16:26 2011

Re: Too much options in LyX

On 29-4-2011 15:03, venom00 wrote:
>> Style and form comments are always welcome but please give a 
>> little attention to the content too:
>> - Alternative ideas to highlight matching widgets? Currently 
>> they become red.
>> - Ideas on how to make the research faster? In the next patch 
>> I'll try to add a little delay.
> 
> Here's the new patch. I've added a 300 ms delay, now it should be more
> responsive as the search it's not done for each charatecter in input.
> 
> venom00
> 
> Index: src/frontends/qt4/GuiDocument.h
> ===================================================================
> --- src/frontends/qt4/GuiDocument.h	(revisione 38474)
> +++ src/frontends/qt4/GuiDocument.h	(copia locale)
>  <at>  <at>  -77,6 +77,7  <at>  <at> 
>  	void updateDefaultFormat();
>  	void updatePagestyle(std::string const &, std::string const &);
>  	bool isChildIncluded(std::string const &);
> +	void hideView();
>  
>  	///
>  	BufferParams const & params() const { return bp_; }
> Index: src/frontends/qt4/GuiDocument.cpp
> ===================================================================
> --- src/frontends/qt4/GuiDocument.cpp	(revisione 38474)
> +++ src/frontends/qt4/GuiDocument.cpp	(copia locale)
>  <at>  <at>  -2139,6 +2139,12  <at>  <at> 
>  			  includeonlys_.end(), child) != includeonlys_.end());
>  }
>
> +void GuiDocument::hideView()
> +{
> +	Dialog::hideView();
> +	// Reset the search box
> +	this->docPS->resetSearch();
> +}
>
>  void GuiDocument::applyView()
>  {

hideView() doesn't work as this is only called when the Ok or Close
button are pressed. Not when the dialog is canceled by Escape.

Maybe you should put the resetSearch function in a showEvent function
which is called when the dialog is shown.

However, you should then set the text in the textfield and then
manually reset all widgets to enabled. This means you have to either
block the signals, but better .. listen to textEdited instead of
textChanged.

Style: remove the this-> and the comment is not necessary: resetSearch()
is clear enough to me.

> +
> +	connect(search_, SIGNAL(textChanged(QString)), this,
> SLOT(filterChanged(QString)));

textChanged -> textEdited so that this will not react to
programmatically changing the text in the search box as a
consequence of resetSearch() for instance.

>  <at>  <at>  -132,22 +163,138  <at>  <at> 
>  			     QTreeWidgetItem * previous)
>  {
>  	// do nothing when clicked on whitespace (item=NULL)
> -	if( !item )
> +	if(!item)
>  		return;

I believe this comes from my first patch to LyX some 2.5 years ago,
but if you care about the whitespaces there should also be a space
between if and (.

>  
> +bool matches(QString & input, QString const & search)

QString const & input ... ?

> +{
> +	// Check if the input contains the search string
> +	return input.remove('&').contains(search, Qt::CaseInsensitive);
> +}
>  

> +void PanelStack::searchTimeout()

I would like it more that searchTimeout just calls
search() which then implements the searching.

Then you can call search() from showEvent as well for instance.

> +{
> +	QString search = search_->text();
> +	bool enable_all = search.isEmpty();
> +
> +	// If the search string is empty we renable all the items

renable -> enable

> +	// otherwise we disable everything and then selectively
> +	// re-enable matching items
> +	foreach (QTreeWidgetItem * tree_item, panel_map_) {
> +		setTreeItemStatus(tree_item, enable_all);
> +	}

if enable_all is true, we don't need to loop over all widgets...

> +
> +	foreach (QTreeWidgetItem * tree_item, panel_map_) {
> +
> +		// Current widget
> +		QWidget * pane_widget = widget_map_[tree_item];
> +
> +		// First of all we look in the pane name
> +		bool pane_matches = tree_item->text(0).contains(search,
> Qt::CaseInsensitive);
> +
> +		// If the tree item has an associated pane
> +		if (pane_widget) {
>		[..]
> +> +		}
> +
> +	}
> +}
> +

Vincent

BH | 1 May 16:28 2011
Picon

Compile error in branch

I haven't tried compiling branch in a while, but trying today leads to
a compile error (using Qt-4.4):

In file included from SignalSlotPrivate.cpp:15:
SignalSlotPrivate_moc.cpp:14:2: error: #error "This file was generated
using the moc from 4.7.1. It"
SignalSlotPrivate_moc.cpp:15:2: error: #error "cannot be used with the
include files from this version of Qt."
SignalSlotPrivate_moc.cpp:16:2: error: #error "(The moc has changed too much.)"
In file included from SignalSlotPrivate.cpp:15:
SignalSlotPrivate_moc.cpp: In member function ‘virtual const
QMetaObject* lyx::SignalImpl::metaObject() const’:
SignalSlotPrivate_moc.cpp:54: error: ‘class QObjectData’ has no member
named ‘metaObject’
SignalSlotPrivate_moc.cpp:54: error: ‘class QObjectData’ has no member
named ‘metaObject’
SignalSlotPrivate_moc.cpp: In member function ‘virtual const
QMetaObject* lyx::SlotImpl::metaObject() const’:
SignalSlotPrivate_moc.cpp:119: error: ‘class QObjectData’ has no
member named ‘metaObject’
SignalSlotPrivate_moc.cpp:119: error: ‘class QObjectData’ has no
member named ‘metaObject’
make[5]: *** [SignalSlotPrivate.lo] Error 1

BH

Jürgen Spitzmüller | 1 May 16:32 2011

Re: Compile error in branch

BH wrote:
> I haven't tried compiling branch in a while, but trying today leads to
> a compile error (using Qt-4.4):

Did you try with a make distclean'ed tree?

Jürgen

Vincent van Ravesteijn | 1 May 16:33 2011

Re: Compile error in branch

On 1-5-2011 16:28, BH wrote:
> I haven't tried compiling branch in a while, but trying today leads to
> a compile error (using Qt-4.4):

This is the error:

"This file was generated using the moc from 4.7.1. It
cannot be used with the include files from this version of Qt.
(The moc has changed too much.)"

When LyX is compiled Qt will create files like:

src/frontends/qt4/GuiAbout_moc.cpp.

Apparently you previously compiled with version Qt 4.7.1 and the moc
files are still there. Please remove them, so that they get regenerated
with Qt 4.4. again.

Vincent

Richard Heck | 1 May 16:55 2011
Picon
Picon

Re: Memory LyXs

On 04/30/2011 09:14 PM, venom00 wrote:
>> I've looked briefly at this one before, and I find it confusing.  It
>> looks as if it is saying that we are leaking the docstring 
>> word itself,
>> i.e., that it's not being cleaned up, but why would that be?
> Mmmh, I find it pretty simple: we have some elements added to a set but never removed.
I know less about this then I should. (I'm just an amateur.) But: When
we call delete d, doesn't this call the d->words_ destructor? I mean,
you can do:

    if (...) {
        set<int> tmp;
        for (int i = 0; i < 1000; ++i)
            tmp.insert(i);
    }

and there is no memory leak, because all the ints in the set are
destroyed when tmp goes out of scope. Similarly, when we delete the
pimpl, the words_ set should be deleted, and this takes care of
everything in that set. Right?

>> The other thing maybe worth trying is something simple like:
>>
>> Index: src/Paragraph.cpp
>> ===================================================================
>> --- src/Paragraph.cpp (revision 38560)
>> +++ src/Paragraph.cpp (working copy)
>>  <at>  <at>  -3500,12 +3500,11  <at>  <at> 
>>     pos_type from = pos;
>>     locateWord(from, pos, WHOLE_WORD);
>>     if (pos - from >= minlength) {
>> -     docstring word = asString(from, pos, AS_STR_NONE);
>>       FontList::const_iterator cit = d->fontlist_.fontIterator(pos);
>>       if (cit == d->fontlist_.end())
>> -       return;
>> +       break;
>>       Language const * lang = cit->font().language();
>> -     d->words_[*lang].insert(word);
>> +     d->words_[*lang].insert(asString(from, pos, AS_STR_NONE););
>>     }
>>   }
>>  }
> set<T>.insert() stores a copy [2] so you have to remove it manually. word should die when goes out of scope.
>
Yes, I know it *should* die, but I was wondering if maybe it didn't for
some odd reason.

rh

venom00 | 1 May 22:37 2011

RE: Too much options in LyX

> hideView() doesn't work as this is only called when the Ok or Close
> button are pressed. Not when the dialog is canceled by Escape.
> 
> Maybe you should put the resetSearch function in a showEvent function
> which is called when the dialog is shown.

OK, that's a good idea, this makes the patch more self-contained, but I'll use
hideEvent. showEvent is called after the widget is shown, that's not what we
want.

> However, you should then set the text in the textfield and then
> manually reset all widgets to enabled. This means you have to either
> block the signals, but better .. listen to textEdited instead of
> textChanged.
> > +
> > +	connect(search_, SIGNAL(textChanged(QString)), this,
> > SLOT(filterChanged(QString)));
> 
> 
> textChanged -> textEdited so that this will not react to
> programmatically changing the text in the search box as a
> consequence of resetSearch() for instance.

Why we don't want that? It's correct: "search_->setText(QString())" re-enables
all the widgets automatically. See below.

> > +bool matches(QString & input, QString const & search)
> 
> QString const & input ... ?
> > +{
> > +	// Check if the input contains the search string
> > +	return input.remove('&').contains(search, Qt::CaseInsensitive);
> > +}

QString & QString::remove ( QChar ch, Qt::CaseSensitivity cs = Qt::CaseSensitive
) <-- Not const method. Actually removes the char from the string.

> > +void PanelStack::searchTimeout()
> 
> I would like it more that searchTimeout just calls
> search() which then implements the searching.
> 
> Then you can call search() from showEvent as well for instance.

Right.

> > +	// otherwise we disable everything and then selectively
> > +	// re-enable matching items
> > +	foreach (QTreeWidgetItem * tree_item, panel_map_) {
> > +		setTreeItemStatus(tree_item, enable_all);
> > +	}
> 
> if enable_all is true, we don't need to loop over all widgets...

We have to, enable_all (search box empty) means we have to re-enable all the
items in the tree, as they could have been disabled by the previous search.

Thanks for your comments.
venom00

Index: src/frontends/qt4/PanelStack.cpp
===================================================================
--- src/frontends/qt4/PanelStack.cpp	(revisione 38474)
+++ src/frontends/qt4/PanelStack.cpp	(copia locale)
 <at>  <at>  -15,12 +15,23  <at>  <at> 
 #include "qt_helpers.h"

 #include "support/debug.h"
+#include "support/foreach.h"

+#include <QApplication>
+#include <QAbstractButton>
+#include <QColorGroup>
+#include <QComboBox>
 #include <QFontMetrics>
+#include <QGroupBox>
 #include <QHBoxLayout>
 #include <QHeaderView>
+#include <QLabel>
+#include <QLineEdit>
+#include <QListWidget>
+#include <QPalette>
 #include <QStackedWidget>
 #include <QTreeWidget>
+#include <QVBoxLayout>

 #include "support/lassert.h"

 <at>  <at>  -33,9 +44,16  <at>  <at> 
 PanelStack::PanelStack(QWidget * parent)
 	: QWidget(parent)
 {
+	delay_search_ = new QTimer(this);
 	list_ = new QTreeWidget(this);
 	stack_ = new QStackedWidget(this);
+	search_ = new QLineEdit(this);

+	// Configure the timer
+	delay_search_->setSingleShot(true);
+	connect(delay_search_, SIGNAL(timeout()), this, SLOT(searchTimeout()));
+
+	// Configure tree
 	list_->setRootIsDecorated(false);
 	list_->setColumnCount(1);
 	list_->header()->hide();
 <at>  <at>  -48,9 +66,22  <at>  <at> 
 	connect(list_, SIGNAL(itemClicked (QTreeWidgetItem*, int)),
 		this, SLOT(itemSelected(QTreeWidgetItem *, int)));

-	QHBoxLayout * layout = new QHBoxLayout(this);
-	layout->addWidget(list_, 0);
-	layout->addWidget(stack_, 1);
+	// Configure the search box
+#if QT_VERSION >= 0x040700
+	search_->setPlaceholderText(qt_("Search"));
+#endif
+
+	connect(search_, SIGNAL(textChanged(QString)), this,
SLOT(filterChanged(QString)));
+
+	// Create the output layout, horizontal plus a VBox on the left with the
search
+	// box and the tree
+	QVBoxLayout * left_layout = new QVBoxLayout();
+	left_layout->addWidget(search_, 0);
+	left_layout->addWidget(list_, 1);
+
+	QHBoxLayout * main_layout = new QHBoxLayout(this);
+	main_layout->addLayout(left_layout, 0);
+	main_layout->addWidget(stack_, 1);
 }

 
 <at>  <at>  -132,22 +163,152  <at>  <at> 
 			     QTreeWidgetItem * previous)
 {
 	// do nothing when clicked on whitespace (item=NULL)
-	if( !item )
+	if (!item)
 		return;

 	// if we have a category, expand the tree and go to the
-	// first item
+	// first enabled item
 	if (item->childCount() > 0) {
 		item->setExpanded(true);
-		if (previous && previous->parent() != item)
-			switchPanel( item->child(0), previous );
+		if (previous && previous->parent() != item) {
+			// Looks for a child not disabled
+			for (int i = 0; i < item->childCount(); ++i) {
+				if (item->child(i)->flags() & Qt::ItemIsEnabled)
{
+					switchPanel(item->child(i), previous);
+					break;
+				}
+			}
+		}
 	}
 	else if (QWidget * w = widget_map_.value(item, 0)) {
 		stack_->setCurrentWidget(w);
 	}
 }

+bool matches(QString & input, QString const & search)
+{
+	// Check if the input contains the search string
+	return input.remove('&').contains(search, Qt::CaseInsensitive);
+}

+void setTreeItemStatus(QTreeWidgetItem * tree_item, bool enabled)
+{
+	// Enable/disable the item
+	tree_item->setDisabled(!enabled);
+
+	// Change the color from black to gray or viceversa
+	QPalette::ColorGroup new_color = enabled ? QPalette::Active :
QPalette::Disabled;
+	tree_item->setTextColor(0, QApplication::palette().color(new_color,
QPalette::Text));
+}
+
+void PanelStack::hideEvent(QHideEvent * event)
+{
+	QWidget::hideEvent(event);
+
+	// Programatically hidden (not simply minimized by the user)
+	if (!event->spontaneous())
+		resetSearch();
+}
+
+void PanelStack::filterChanged(QString const & /*search*/)
+{
+	// The text in the search box is changed, reset the timer
+	// and then search in the widgets
+	delay_search_->start(300);
+}
+
+void PanelStack::searchTimeout()
+{
+	search();
+}
+
+void PanelStack::search()
+{
+	QString search = search_->text();
+	bool enable_all = search.isEmpty();
+
+	// If the search string is empty we enable all the items
+	// otherwise we disable everything and then selectively
+	// re-enable matching items
+	foreach (QTreeWidgetItem * tree_item, panel_map_) {
+		setTreeItemStatus(tree_item, enable_all);
+	}
+
+	foreach (QTreeWidgetItem * tree_item, panel_map_) {
+
+		// Current widget
+		QWidget * pane_widget = widget_map_[tree_item];
+
+		// First of all we look in the pane name
+		bool pane_matches = tree_item->text(0).contains(search,
Qt::CaseInsensitive);
+
+		// If the tree item has an associated pane
+		if (pane_widget) {
+
+			// Loops on the list of children widgets (recursive)
+			QWidgetList children = pane_widget->findChildren<QWidget
*>();
+			foreach (QWidget * child_widget, children) {
+				bool widget_matches = false;
+
+				// Try to cast to the most common widgets and
looks in it's content
+				// It's bad OOP, it would be nice to have a
QWidget::toString() overloaded by
+				// each widget, but this would require to change
Qt or subclass each widget.
+				// Note that we have to ignore the amperstand
symbol
+				if (QAbstractButton * button =
qobject_cast<QAbstractButton *>(child_widget)) {
+					widget_matches = matches(button->text(),
search);
+
+				} else if (QGroupBox * group_box =
qobject_cast<QGroupBox *>(child_widget)) {
+					widget_matches =
matches(group_box->title(), search);
+
+				} else if (QLabel * label = qobject_cast<QLabel
*>(child_widget)) {
+					widget_matches = matches(label->text(),
search);
+
+				} else if (QLineEdit * line_edit =
qobject_cast<QLineEdit *>(child_widget)) {
+					widget_matches =
matches(line_edit->text(), search);
+
+				} else if (QListWidget * list_widget =
qobject_cast<QListWidget *>(child_widget)) {
+					widget_matches =
(list_widget->findItems(search, Qt::MatchContains)).count() > 0;
+
+				} else if (QTreeWidget * tree_view =
qobject_cast<QTreeWidget *>(child_widget)) {
+					widget_matches =
(tree_view->findItems(search, Qt::MatchContains)).count() > 0;
+
+				} else if (QComboBox * combo_box =
qobject_cast<QComboBox *>(child_widget)) {
+					widget_matches =
(combo_box->findText(search, Qt::MatchContains)) != -1;
+
+				} else {
+					continue;
+				}
+
+				// If this widget meets the search criteria
+				if (widget_matches && !enable_all) {
+					// The pane too meets the search
criteria
+					pane_matches = true;
+
+					// Highlight the widget
+					QPalette widget_palette =
child_widget->palette();
+
widget_palette.setColor(child_widget->foregroundRole(), Qt::red);
+
child_widget->setPalette(widget_palette);
+				} else {
+					// Reset the color of the widget
+
child_widget->setPalette(QApplication::palette(child_widget));
+				}
+			}
+
+			// If the pane meets the search criteria
+			if (pane_matches && !enable_all) {
+				// Expand and enable the pane and his ancestors
(typically just the parent)
+				QTreeWidgetItem * item = tree_item;
+				do {
+					item->setExpanded(true);
+					setTreeItemStatus(item, true);
+					item = item->parent();
+				} while (item);
+			}
+		}
+
+	}
+}
+
 void PanelStack::itemSelected(QTreeWidgetItem * item, int)
 {
 	// de-select the category if a child is selected
 <at>  <at>  -162,6 +323,11  <at>  <at> 
 		qMax(list_->height(), stack_->height()));
 }

+void PanelStack::resetSearch()
+{
+	search_->setText(QString());
+}
+
 } // namespace frontend
 } // namespace lyx

Index: src/frontends/qt4/PanelStack.h
===================================================================
--- src/frontends/qt4/PanelStack.h	(revisione 38474)
+++ src/frontends/qt4/PanelStack.h	(copia locale)
 <at>  <at>  -13,12 +13,16  <at>  <at> 
 #ifndef PANELSTACK_H
 #define PANELSTACK_H

+#include <QHash>
+#include <QHideEvent>
+#include <QTimer>
 #include <QWidget>
-#include <QHash>

+class QAbstractButton;
+class QLineEdit;
+class QStackedWidget;
 class QTreeWidget;
 class QTreeWidgetItem;
-class QStackedWidget;

 namespace lyx {
 namespace frontend {
 <at>  <at>  -44,13 +48,25  <at>  <at> 
 	bool isCurrentPanel(QString const & name) const;
 	///
 	QSize sizeHint() const;
+	/// perform the search
+	void search();
+	/// reset the search box
+	void resetSearch();

 public Q_SLOTS:
+	/// the option filter changed
+	void filterChanged(QString const & search);
+	/// timeout for the search box
+	void searchTimeout();
 	/// set current panel from an item
 	void switchPanel(QTreeWidgetItem * it, QTreeWidgetItem * previous = 0);
 	/// click on the tree
 	void itemSelected(QTreeWidgetItem *, int);

+protected:
+	/// widget hidden
+	void hideEvent(QHideEvent * event);
+
 private:
 	///
 	typedef QHash<QString, QTreeWidgetItem *> PanelMap;
 <at>  <at>  -61,11 +77,18  <at>  <at> 

 	WidgetMap widget_map_;

+	/// contains the search box
+	QLineEdit * search_;
+
 	/// contains the items
 	QTreeWidget * list_;

 	/// contains the panes
 	QStackedWidget * stack_;
+
+	// timer to delay the search between options
+	QTimer * delay_search_;
+
 };

 } // namespace frontend

Gmane