Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: rework everything to use wxString instead of pointer-to-wxChar #112

Closed
wants to merge 1 commit into from
Closed

Conversation

@jengelh
Copy link

@jengelh jengelh commented Feb 27, 2016

make it build with wx3 --enable-stl and throw out other type sillynesses. That's all. It's not tested.

  • abolish reliance on pointer-to-wxChar
  • use backend-neutral wxString
  • drop all wxString::c_str() calls because they give a pointer to
    the internal representation, which is also bounded by the lifetime
    of the surrounding object.
  • drop calls to wxString::mb_str() because they give a
    platform-dependent internal representation; under a Unicode
    wxWidgets build, this is wchar_t, which
    • on Windows: is not usable, because most APIs expect narrow
      locale-specific strings
    • on Linux: is not usable, because most APIs expect narrow
      UTF-8 strings
  • drop calls to wx_str, same as mb_str
* abolish reliance on pointer-to-wxChar
* use backend-neutral wxString
* drop all wxString::c_str() calls because they give a pointer to
  the internal representation, which is also bounded by the lifetime
  of the surrounding object.
* drop calls to wxString::mb_str() because they give a
  platform-dependent internal representation; under a Unicode
  wxWidgets build, this is wchar_t, which
  - on Windows: is not usable, because most APIs expect narrow
    locale-specific strings
  - on Linux: is not usable, because most APIs expect narrow
    UTF-8 strings
* drop calls to wx_str, same as mb_str
@Paul-Licameli
Copy link
Member

@Paul-Licameli Paul-Licameli commented Feb 27, 2016

A brief but not complete look shows you were alert to use const wString& in
arguments. That is good. I lately made my own sweep replacing string
value arguments with references, so let's not get more of that again.

Did you make some attempt to stop at least once in every changed function
and be sure nothing funny happens?
PRL

On Friday, February 26, 2016, jengelh notifications@github.com wrote:

make it build with wx3 --enable-stl and throw out other type sillynesses.
That's all. It's not tested.

abolish reliance on pointer-to-wxChar
use backend-neutral wxString
drop all wxString::c_str() calls because they give a pointer to the
internal representation, which is also bounded by the lifetime of the
surrounding object.
drop calls to wxString::mb_str() because they give a platform-dependent
internal representation; under a Unicode wxWidgets build, this is wchar_t,
which

on Windows: is not usable, because most APIs expect narrow
locale-specific strings
on Linux: is not usable, because most APIs expect narrow UTF-8 strings

drop calls to wx_str, same as mb_str


You can view, comment on, or merge this pull request online at:

#112

Commit Summary

build: rework everything to use wxString instead of pointer-to-wxChar

File Changes

M .gitignore (9)
M src/AboutDialog.cpp (4)
M src/AboutDialog.h (4)
M src/AudacityApp.cpp (39)
M src/AutoRecovery.cpp (77)
M src/AutoRecovery.h (7)
M src/BatchCommands.cpp (16)
M src/BatchProcessDialog.cpp (6)
M src/Benchmark.cpp (4)
M src/BlockFile.cpp (4)
M src/DirManager.cpp (127)
M src/DirManager.h (10)
M src/Envelope.cpp (13)
M src/Envelope.h (14)
M src/FFT.cpp (2)
M src/FFT.h (2)
M src/FFmpeg.cpp (77)
M src/FFmpeg.h (4)
M src/FileIO.cpp (4)
M src/FileNames.cpp (2)
M src/FreqWindow.cpp (14)
M src/Internat.cpp (7)
M src/Internat.h (4)
M src/LabelDialog.cpp (4)
M src/LabelTrack.cpp (29)
M src/LabelTrack.h (4)
M src/LangChoice.cpp (8)
M src/Languages.cpp (12)
M src/Legacy.cpp (2)
M src/Menus.cpp (34)
M src/ModuleManager.cpp (17)
M src/NoteTrack.cpp (19)
M src/NoteTrack.h (4)
M src/PitchName.cpp (77)
M src/PitchName.h (4)
M src/PluginManager.cpp (36)
M src/PluginManager.h (1)
M src/Prefs.cpp (2)
M src/Project.cpp (77)
M src/Project.h (10)
M src/SampleFormat.cpp (2)
M src/SampleFormat.h (2)
M src/SelectedRegion.cpp (14)
M src/SelectedRegion.h (14)
M src/Sequence.cpp (60)
M src/Sequence.h (8)
M src/Tags.cpp (18)
M src/Tags.h (4)
M src/Theme.cpp (44)
M src/TimeTrack.cpp (18)
M src/TimeTrack.h (6)
M src/TimerRecordDialog.cpp (4)
M src/TrackPanel.cpp (34)
M src/ViewInfo.cpp (2)
M src/ViewInfo.h (2)
M src/WaveClip.cpp (12)
M src/WaveClip.h (6)
M src/WaveTrack.cpp (17)
M src/WaveTrack.h (6)
M src/blockfile/LegacyAliasBlockFile.cpp (12)
M src/blockfile/LegacyAliasBlockFile.h (2)
M src/blockfile/LegacyBlockFile.cpp (14)
M src/blockfile/LegacyBlockFile.h (2)
M src/blockfile/ODDecodeBlockFile.cpp (12)
M src/blockfile/ODDecodeBlockFile.h (2)
M src/blockfile/ODPCMAliasBlockFile.cpp (12)
M src/blockfile/ODPCMAliasBlockFile.h (2)
M src/blockfile/PCMAliasBlockFile.cpp (12)
M src/blockfile/PCMAliasBlockFile.h (2)
M src/blockfile/SilentBlockFile.cpp (12)
M src/blockfile/SilentBlockFile.h (2)
M src/blockfile/SimpleBlockFile.cpp (12)
M src/blockfile/SimpleBlockFile.h (2)
M src/commands/CommandManager.cpp (88)
M src/commands/CommandManager.h (44)
M src/commands/HelpCommand.cpp (2)
M src/commands/ImportExportCommands.cpp (6)
M src/commands/ResponseQueue.h (2)
M src/commands/ScreenshotCommand.cpp (2)
M src/effects/ChangePitch.cpp (2)
M src/effects/ChangeSpeed.cpp (2)
M src/effects/Contrast.cpp (4)
M src/effects/DtmfGen.cpp (6)
M src/effects/DtmfGen.h (2)
M src/effects/Effect.cpp (18)
M src/effects/Effect.h (4)
M src/effects/EffectManager.cpp (4)
M src/effects/EffectRack.cpp (4)
M src/effects/Equalization.cpp (32)
M src/effects/Equalization.h (5)
M src/effects/Equalization48x.cpp (4)
M src/effects/LoadEffects.cpp (4)
M src/effects/Noise.cpp (2)
M src/effects/NoiseReduction.cpp (16)
M src/effects/Reverb.cpp (4)
M src/effects/ScienFilter.cpp (4)
M src/effects/TruncSilence.cpp (2)
M src/effects/VST/VSTEffect.cpp (104)
M src/effects/VST/VSTEffect.h (8)
M src/effects/audiounits/AudioUnitEffect.cpp (36)
M src/effects/ladspa/LadspaEffect.cpp (6)
M src/effects/nyquist/LoadNyquist.cpp (4)
M src/effects/nyquist/Nyquist.cpp (96)
M src/export/Export.cpp (10)
M src/export/Export.h (2)
M src/export/ExportCL.cpp (8)
M src/export/ExportFFmpeg.cpp (18)
M src/export/ExportFFmpegDialogs.cpp (84)
M src/export/ExportFFmpegDialogs.h (30)
M src/export/ExportFLAC.cpp (6)
M src/export/ExportMP2.cpp (4)
M src/export/ExportMP3.cpp (20)
M src/export/ExportMultiple.cpp (16)
M src/export/ExportOGG.cpp (4)
M src/export/ExportPCM.cpp (190)
M src/import/Import.cpp (64)
M src/import/ImportFFmpeg.cpp (12)
M src/import/ImportFLAC.cpp (2)
M src/import/ImportGStreamer.cpp (38)
M src/import/ImportLOF.cpp (2)
M src/import/ImportMIDI.cpp (2)
M src/import/ImportMP3.cpp (2)
M src/import/ImportOGG.cpp (4)
M src/import/ImportPlugin.h (2)
M src/import/ImportQT.cpp (8)
M src/import/ImportRaw.cpp (2)
M src/ondemand/ODComputeSummaryTask.h (2)
M src/ondemand/ODDecodeTask.h (2)
M src/ondemand/ODTask.h (2)
M src/ondemand/ODWaveTrackTaskQueue.cpp (4)
M src/prefs/DirectoriesPrefs.cpp (6)
M src/prefs/KeyConfigPrefs.cpp (10)
M src/prefs/MidiIOPrefs.cpp (12)
M src/prefs/ModulePrefs.cpp (2)
M src/toolbars/ControlToolBar.cpp (2)
M src/toolbars/ControlToolBar.h (2)
M src/toolbars/EditToolBar.cpp (2)
M src/toolbars/EditToolBar.h (2)
M src/toolbars/SelectionBar.cpp (8)
M src/toolbars/SelectionBar.h (2)
M src/toolbars/ToolBar.cpp (2)
M src/toolbars/ToolsToolBar.cpp (4)
M src/toolbars/ToolsToolBar.h (6)
M src/toolbars/TranscriptionToolBar.cpp (2)
M src/toolbars/TranscriptionToolBar.h (2)
M src/widgets/ASlider.cpp (4)
M src/widgets/HelpSystem.cpp (4)
M src/widgets/KeyView.cpp (24)
M src/widgets/Meter.cpp (4)
M src/widgets/MultiDialog.cpp (19)
M src/widgets/MultiDialog.h (2)
M src/widgets/NumericTextCtrl.cpp (10)
M src/widgets/Ruler.cpp (4)
M src/widgets/numformatter.cpp (2)
M src/xml/XMLFileReader.cpp (4)
M src/xml/XMLTagHandler.cpp (18)
M src/xml/XMLTagHandler.h (6)
M src/xml/XMLWriter.cpp (33)
M src/xml/XMLWriter.h (1)

Patch Links:

https://github.com/audacity/audacity/pull/112.patch
https://github.com/audacity/audacity/pull/112.diff


Reply to this email directly or view it on GitHub.<
https://ci6.googleusercontent.com/proxy/vpvJrEFOLM1wcJ8n05WUo-XKdZslV8L5AMu_zKkds3jQrzJX23vRA8SGXCZwwslvopun-pWcZUw9oVMFsRIUYLOVj5e2AK_V-QqWDoDgBBsYJyrppGennaKEJfj8L2-lVSF4_46ebSnYSyzErBCLGkU9CqHEiQ=s0-d-e1-ft#https://github.com/notifications/beacon/ALITYW977OW98_-fw_alHtooqWRhxH4dks5poPEGgaJpZM4HkUue.gif

@ThomasFeher
Copy link
Contributor

@ThomasFeher ThomasFeher commented Mar 12, 2016

This fixes building on openSuse. 👍

@Paul-Licameli
Copy link
Member

@Paul-Licameli Paul-Licameli commented Mar 12, 2016

I am not willing to pull this. This is a large commit that Jan admitted
was not tested. I may not understand all the issues with multibyte
characters and such. I don't know who else can review.

Better to break this into smaller commits, for instance such as separating
all the changes for XML handling.

I noticed a profanity in one of the comments -- please don't do that.

PRL

On Sat, Mar 12, 2016 at 11:10 AM, ThomasFeher notifications@github.com
wrote:

This fixes building on openSuse. [image: 👍]


Reply to this email directly or view it on GitHub
#112 (comment).

@JamesCrook
Copy link
Member

@JamesCrook JamesCrook commented Sep 9, 2016

Leaving this open for now, even though now has conflicts. I think the idea is right. The changes should be 'mechanical' and hence safe.

@JamesCrook
Copy link
Member

@JamesCrook JamesCrook commented Sep 12, 2016

Added reject-if-not-updated soon. It has merge conflicts, a profanity, and Paul would like it in smaller chunks. It's still a nice idea, and would help us on SUSE.

Copy link
Contributor

@richardash1981 richardash1981 left a comment

There is quite a mixture in here. The varags function changes look odd as they are the opposite of what worked in wx2.8, but match the documentation (and the one I tried worked for me).

.dirstamp
stamp-h[0-9]*
Makefile
Makefile.in

This comment has been minimized.

@richardash1981

richardash1981 Sep 19, 2016
Contributor

We currently track Makefile.in. Don't want this change mixed in with charset changes.

else
printf("ASSERTION FAILED!\n%s: %d\n", (const char *)wxString(fileName).mb_str(), lineNumber);
wxPrintf("ASSERTION FAILED!\n%s: %d\n", fileName, lineNumber);

This comment has been minimized.

@richardash1981

richardash1981 Sep 19, 2016
Contributor

This is an example of the change described here: http://docs.wxwidgets.org/trunk/classwx_string.html#string_vararg which basically boils down to the "right" solution being different to what worked in wx2.8.

@JamesCrook
Copy link
Member

@JamesCrook JamesCrook commented Jul 13, 2018

No activity since 2016. Closing.

@JamesCrook JamesCrook closed this Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants