tags 748634 + patch
thanks
On Tue, Jun 03, 2014 at 12:39:03AM +0100, Deivi San wrote:
> Dear maintainer and dear Olly,
>
> I've tried my best to contribue my two cents in migrating to
> wxwidgets3.0. The project just seemed at my hand reach.
Thanks for the patch. Generally it looks good, but a few comments:
> - GetDocument()->GetPrintableName(name);
> + /*DVD GetDocument()->GetPrintableName(name);*/
I wouldn't leave in the old code commented out like this - the patch
itself documents what the code was before the change.
> + name=GetDocument()->GetUserReadableName();
> wxString title = wxT("Chip's Workshop");
> - if(name)
> - title += wxT(" - ") + name;
> + /* Not needed any more GURN returns default wxString unnamed
> + if(name)
> + */
FWIW, wx 2.8 allows:
if (somewxstring) ...
But in wx 3.0 you have to write this (which also works with 2.8):
if (!somewxstring.empty()) ...
But if in this case an empty string is never returned, the test isn't
needed.
> const wxWX2MBbuf str = dlg.title.mb_str(wxConvISO8859_1);
> - if(str != NULL)
> +/*DVD mb_str never returns NULL as of wx/string.h:AsChar
> if(str != NULL)*/
I think mb_str() now returns an empty string if the conversion fails, so
you should probably check something like this here and in other places
where previously a NULL check was used to detect if the conversion
failed:
if (!str.empty() || dlg.title.empty())
> @@ -81,7 +81,13 @@
> wxMenuItem* item = *itemit;
> if(item == NULL)
> continue;
> - item->SetText(GetItemLabel(i, newitems, emptytext));
> +
> +/*DVD item->SetText(GetItemLabel(i, newitems, emptytext));*/
> +#if wxCHECK_VERSION(2,8,0)
> + item->SetItemLabel(GetItemLabel(i, newitems, emptytext));
Shouldn't that be "2,9,0"? Otherwise it's true for wx >= 2.8.0. Not an
issue for updating the Debian package, but if you're going to
conditionalise on the wx version, you ought to have the version check
right.
Have you rebuilt chipw using this patch and tested it works properly?
Cheers,
Olly
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
