#11372: wxLongLongFmtSpec broken in 2.9.1
------------------------------------------------+---------------------------
Reporter: catalin | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: base | Version: 2.9-svn
Keywords: wxLongLongFmtSpec wxT wchar_t char | Blockedby:
Patch: 0 | Blocking:
------------------------------------------------+---------------------------
Added a patch to the minimal sample to reproduce it.
I used mingw. The error reported:
cannot convert 'const wchar_t*' to 'const char*' for argument '1' to 'int
printf(const char*, ...)'.
It seems that wxT() macro should no longer be used but I may be wrong.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372>
#11372: wxLongLongFmtSpec broken in 2.9.1
------------------------------------------------+---------------------------
Reporter: catalin | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: base | Version: 2.9-svn
Keywords: wxLongLongFmtSpec wxT wchar_t char | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Changes (by catalin):
* patch: 0 => 1
Comment:
Although there was no review/advice for this ticket yet I made a patch
that makes wxLongLongFmtSpec usable again.
I can also change it in case this is not the best approach.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:1>
#11372: wxLongLongFmtSpec should be normal and not wide string
------------------------------------------------+---------------------------
Reporter: catalin | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 2.9.1
Component: base | Version: 2.9-svn
Keywords: wxLongLongFmtSpec wxT wchar_t char | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Changes (by vadz):
* status: new => confirmed
* milestone: => 2.9.1
Comment:
Removing `wxT()` from the definition of wxLongLongFmtSpec is indeed the
right thing to do (this will make it compatible with the old ANSI build at
the price of making it incompatible with the old Unicode build -- but this
is the choice which we consistently made in all the other similar cases).
However I don't think that expressions like {{{ wxT("%" wxLongLongFmtSpec
"x") }}} can compile when wxT() is defined non-trivially. It should be
really just {{{ "%" wxLongLongFmtSpec "x" }}} and the type of the variable
it's being assigned to should be changed to be `char*` instead of
`wxChar*`.
Could you please update the patch to do this? If you could also mention
this change in `docs/changes.txt` it would be great.
TIA!
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:2>
#11372: wxLongLongFmtSpec should be normal and not wide string
------------------------------------------------+---------------------------
Reporter: catalin | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 2.9.1
Component: base | Version: 2.9-svn
Keywords: wxLongLongFmtSpec wxT wchar_t char | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Comment(by catalin):
Replying to [comment:2 vadz]:
> Removing `wxT()` from the definition of wxLongLongFmtSpec is indeed the
right thing to do [...] However I don't think that expressions like {{{
wxT("%" wxLongLongFmtSpec "x") }}} can compile when wxT() is defined non-
trivially.
I suppose it should compile just like {{{ wxT("%") wxLongLongFmtSpec
wxT("x") }}} does now, before applying the patch. With the first patch it
compiles for me - mingw, 2.9.x, [default] Unicode build; I can't really
say about other setups..
> It should be really just {{{ "%" wxLongLongFmtSpec "x" }}} and the type
of the variable it's being assigned to should be changed to be `char*`
instead of `wxChar*`.
This sounds right, but at first I wasn't sure it was the best way. I
changed the patch to remove all wxT() occurrences.
One question: shouldn't this (removal of wxT()) apply about everywhere in
wxW sources for 2.9.x?
> Could you please update the patch to do this?
Done (wxLongLongFmtSpec_fix_2_no-wxT.patch added).
> If you could also mention this change in `docs/changes.txt` it would be
great.
Done.
Regards,
C
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:3>
#11372: wxLongLongFmtSpec should be normal and not wide string
------------------------------------------------+---------------------------
Reporter: catalin | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 2.9.1
Component: base | Version: 2.9-svn
Keywords: wxLongLongFmtSpec wxT wchar_t char | Blockedby:
Patch: 1 | Blocking:
------------------------------------------------+---------------------------
Comment(by vadz):
Replying to [comment:3 catalin]:
> This sounds right, but at first I wasn't sure it was the best way. I
changed the patch to remove all wxT() occurrences.
Thanks, will apply in a moment!
> One question: shouldn't this (removal of wxT()) apply about everywhere
in wxW sources for 2.9.x?
It should be removed in most places but not everywhere. E.g. there is no
harm to keep it in wxMSW implementation: this avoids run-time conversion
from ANSI to Unicode and while they are probably not a big deal, why add
them unnecessarily? For the same (performance) considerations we could
also want to replace it with `wxS()` in some sensitive places.
I.e. it's slightly less trivial than running a global search and replace
for it in the sources.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:4>
#11372: wxLongLongFmtSpec should be normal and not wide string
----------------------+-----------------------------------------------------
Reporter: catalin | Owner:
Type: defect | Status: closed
Priority: normal | Milestone: 2.9.1
Component: base | Version: 2.9-svn
Resolution: fixed | Keywords: wxLongLongFmtSpec wxT wchar_t char
Blockedby: | Patch: 1
Blocking: |
----------------------+-----------------------------------------------------
Changes (by VZ):
* status: confirmed => closed
* resolution: => fixed
Comment:
(In [62562]) Change wxLongLongFmtSpec to be a non-wide string.
As with other ANSI/Unicode unification changes, we choose to preserve
compatibility with the existing code using wxLongLongFmtSpec in ANSI build
and require people using it in Unicode build to change their code.
Closes #11372.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:5>
#11372: wxLongLongFmtSpec should be normal and not wide string
----------------------+-----------------------------------------------------
Reporter: catalin | Owner:
Type: defect | Status: reopened
Priority: normal | Milestone: 2.9.1
Component: base | Version: 2.9-svn
Resolution: | Keywords: wxLongLongFmtSpec wxT wchar_t char
Blockedby: | Patch: 1
Blocking: |
----------------------+-----------------------------------------------------
Changes (by catalin):
* status: closed => reopened
* resolution: fixed =>
Comment:
changes.txt was not updated.
If it was intentionally just close this ticket again.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:6>
#11372: wxLongLongFmtSpec should be normal and not wide string
----------------------+-----------------------------------------------------
Reporter: catalin | Owner:
Type: defect | Status: closed
Priority: normal | Milestone: 2.9.1
Component: base | Version: 2.9-svn
Resolution: fixed | Keywords: wxLongLongFmtSpec wxT wchar_t char
Blockedby: | Patch: 1
Blocking: |
----------------------+-----------------------------------------------------
Changes (by vadz):
* status: reopened => closed
* resolution: => fixed
Comment:
Err, it was changed by the above revision, see
http://trac.wxwidgets.org/changeset/62562/wxWidgets/trunk/docs/changes.txt
Is there anything wrong with this?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:7>
#11372: wxLongLongFmtSpec should be normal and not wide string
----------------------+-----------------------------------------------------
Reporter: catalin | Owner:
Type: defect | Status: closed
Priority: normal | Milestone: 2.9.1
Component: base | Version: 2.9-svn
Resolution: fixed | Keywords: wxLongLongFmtSpec wxT wchar_t char
Blockedby: | Patch: 1
Blocking: |
----------------------+-----------------------------------------------------
Comment(by catalin):
Replying to [comment:7 vadz]:
> Err, it was changed by the above revision, see
http://trac.wxwidgets.org/changeset/62562/wxWidgets/trunk/docs/changes.txt
>
> Is there anything wrong with this?
No, sorry, I may have missed it in the diff.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/11372#comment:8>