[resent since gmane ate the original message yesterday]
Uwe Stöhr wrote:
> >> - docstring const & name = getParam("name");
> >> + string url = to_utf8(getParam("target"));
>
> > This is wrong. String processing must not be done with utf8 encoded
> > strings. It can fail in many ways, for example url.length() may not be
> > equal to the number of characters.
>
> But I don't use a .length() call to avoid this.
But somebody else might do so in the future.
> > If you do any string processing, do it with docstrings, this is safe.
>
> I couldn't get to make .find() work with docstrings.
> Could you implement this?
No time. But docstrings work exactly like std::string, so you should not
need to do more than changing the types string->docstring and
char->char_type.
> > Besides, this is a file format change and needs lyx2lyx support. For
> > example, a previously escaped "\%" needs to be changed to an
> > unescaped "%".
>
> No, the LyX-file itself is not touched, only the LaTeX-output.
Not long ago a change like this required a file format change. Are users now
supposed to do the unescaping manually? Or should they live with the fact
that a file that previously had a "%" in a url now has an additional
backslash?
> > I saw your comment that Richard would do some (other?) lyx2lyx stuff,
> > but not long ago there was a rule that no file format change was
> > allowed to go in without lyx2lyx in the same patch. If this rule has
> > been dropped it was a very bad idea IMHO.
>
> I already committed lyx2lyx stuff: http://www.lyx.org/trac/changeset/20950
That is something else.
> Richard will adopt the covert routine for the layout conversion. So
> basically nothing is broken.
Of course something is broken. Documents produced with the current version
will be misconverted (in one or both directions).
> I think using SVN this way, that somebody is
> allowed to check in something that basically works and it could be fine
> tuned bit by bit later is an advantage. We did e.g. the same with Pavel's
> new hyperref support and I liked that I could fix it bit by bit the next
> days. Of course when we start to provide prereleases, this is not
> suitable.
It used to be always unsuitable.
> > Finally, the code looks very ad hoc, and I would almost bet that it
> > contains a bug.
>
> The find and replace routine is taken from C++ books, so why should it not
> work? Report me a problem, and I'll fix it.
My experience with this (at a first glance simple) kind of stuff is that one
very often introduces bugs when writing it the first time. I did not see a
specific problem. Nevertheless it is never wrong to use existing code
(there are some string replacement functions in support) and not
reinventing the wheel.
> > Why don't you use the existing, bug free escaping code that is used for
> > normal text?
>
> What do you mean?
I meant the code in Paragraph::Pimpl::simpleTeXSpecialChars (it handles some
more characters specially btw). If it can't be used directly it can be
refactored.
> I couldn't find a dialog where the text of the fields is
> treated like text in the work area. Treating the dialog text as we do in
> the work area is only usable for the name field, for the url field, only
> two characters must be escaped, the rest is allowed by \href. There must
> be a reason why this bug was open so long, btw.
Sure. The reason is the file format change.
> But anyway, every
> improvement is welcome!
I won't do more than comment and you are of course free to ignore the
comments if you like.
Georg
