[PATCH 2/2] Support UTF8 encoded tags

[Dirk Hohndel]

I'm about to leave... Maximilian says this fixes the bugs he saw... your
comments seem to imply that it shouldn't

Can the two of you hash this out, please, and create a clean patch
series that has the tag related changes (case insensitive, for example)
separated from those that affect our utf8 string handling?

parse-xml has handled utf8 just fine for a while. It's possible that I
broke this somewhere when removing glib dependencies, but I want to be
really careful before making changes there :->

Thanks

/D

On Thu, 2013-11-14 at 13:13 -0800, Thiago Macieira wrote:
> Details below.
> 
> I'm not sure this is correct. The changes that are trying to fix the bug 
> *should* have had no effect.
> 
> On quinta-feira, 14 de novembro de 2013 19:47:30, Maximilian Güntner wrote:
> > +static void utf8_string(char *buffer, void *_res)
> > +{
> > +	int size;
> > +	char *res;
> > +	while (isspace(*buffer))
> > +		buffer++;
> > +	size = strlen(buffer);
> > +	while (size && isspace(buffer[size-1]))
> > +		size--;
> > +	if (!size)
> > +		return;
> 
> Note that there's a possible bug here. I know you've just moved the code 
> around, so this is something for the future.
> 
> According to the code above, if utf8_string is called on a string containing 
> only spaces, the first loop will advance buffer to the end, so strlen(buffer) == 
> 0. The second loop will never execute because of that, and then we return 
> without setting _res. That means the caller could see garbage.
> 
> > diff --git a/qt-ui/completionmodels.cpp b/qt-ui/completionmodels.cpp
> > index 31733ad..965ce87 100644
> > --- a/qt-ui/completionmodels.cpp
> > +++ b/qt-ui/completionmodels.cpp
> > @@ -43,7 +43,7 @@ void TagCompletionModel::updateModel()
> >  	QStringList list;
> >  	struct tag_entry *current_tag_entry = g_tag_list->next;
> >  	while (current_tag_entry != NULL) {
> > -		list.append(QString(current_tag_entry->tag->name));
> > +		list.append(QString::fromUtf8(current_tag_entry->tag->name));
> 
> This should not be necessary since we set the codec for C strings to UTF-8 in 
> main(). However, this doesn't hurt.
> 
> 
> > diff --git a/qt-ui/maintab.cpp b/qt-ui/maintab.cpp
> > index bd10ed3..7380c72 100644
> > --- a/qt-ui/maintab.cpp
> > +++ b/qt-ui/maintab.cpp
> > @@ -89,6 +89,7 @@ MainTab::MainTab(QWidget *parent) : QTabWidget(parent),
> >  	completers.location = new QCompleter(LocationCompletionModel::instance(),
> > ui.location); 
> > 	completers.suit = new QCompleter(SuitCompletionModel::instance(),
> > ui.suit); 
> > 	completers.tags = new QCompleter(TagCompletionModel::instance(),
> > ui.tagWidget);
> > +	completers.tags->setCaseSensitivity(Qt::CaseInsensitive);
> 
> Unrelated change? At least, this is not explained.
> 
> >  	ui.buddy->setCompleter(completers.buddy);
> >  	ui.divemaster->setCompleter(completers.divemaster);
> >  	ui.location->setCompleter(completers.location);
> > @@ -171,7 +172,7 @@ void MainTab::enableEdition(EditMode newEditMode)
> >  			notesBackup[mydive].datetime = QDateTime::fromTime_t(mydive-
> >when -
> > gettimezoneoffset()).toString(QString("M/d/yy h:mm")); char buf[1024];
> >  			taglist_get_tagstring(mydive->tag_list, buf, 1024);
> > -			notesBackup[mydive].tags = QString(buf);
> > +			notesBackup[mydive].tags = QString::fromUtf8(buf);
> 
> Same thing as before: it shouldn't be necessary.
> 
> > 
> >  			// maybe this is a place for memset?
> >  			for (int i = 0; i < MAX_CYLINDERS; i++) {
> > @@ -393,7 +394,7 @@ void MainTab::updateDiveInfo(int dive)
> > 
> >  		char buf[1024];
> >  		taglist_get_tagstring(d->tag_list, buf, 1024);
> > -		ui.tagWidget->setText(QString(buf));
> > +		ui.tagWidget->setText(QString::fromUtf8(buf));
> 
> Ditto.
> 
> >  		multiEditEquipmentPlaceholder = *d;
> >  		cylindersModel->setDive(&multiEditEquipmentPlaceholder);
> > @@ -695,7 +696,7 @@ void MainTab::saveTags()
> >  		QString tag;
> >  		taglist_clear(mydive->tag_list);
> >  		foreach (tag, ui.tagWidget->getBlockStringList())
> > -			taglist_add_tag(mydive->tag_list, tag.toAscii().data());
> > +			taglist_add_tag(mydive->tag_list, tag.toUtf8().data());
> 
> Ditto for the reverse: toAscii() is documented to be a misnomer and it 
> actually encodes using the codec set for the C strings. In this case, it's 
> UTF-8.
> 
> >  	);
> >  }
> > 
> > diff --git a/save-xml.c b/save-xml.c
> > index 5b0ef42..d16b16d 100644
> > --- a/save-xml.c
> > +++ b/save-xml.c
> > @@ -406,9 +406,9 @@ static void save_tags(FILE *f, struct tag_entry
> > *tag_list) fprintf(f, ", ");
> >  			/* If the tag has been translated, write the source to the xml 
> file */
> >  			if (tmp->tag->source != NULL)
> > -				fprintf(f, "%s", tmp->tag->source);
> > +				quote(f, tmp->tag->source, 0);
> >  			else
> > -				fprintf(f, "%s", tmp->tag->name);
> > +				quote(f, tmp->tag->name, 0);
> 
> Unrelated change?
> 
> >  			tmp = tmp->next;
> >  			more = 1;
> >  		}
> 
> _______________________________________________
> subsurface mailing list
> subsurface at hohndel.org
> http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface