Skip to content

fix remaining build warnings#190

Merged
atsju merged 5 commits intogithubdoe:masterfrom
atsju:JST/warning
May 18, 2025
Merged

fix remaining build warnings#190
atsju merged 5 commits intogithubdoe:masterfrom
atsju:JST/warning

Conversation

@atsju
Copy link
Collaborator

@atsju atsju commented May 17, 2025

This is probably fine. You can release without the changes if you don't want to make a new tag.
I make the PR as draft and will double check with Valgrind.

@atsju atsju requested a review from gr5 May 17, 2025 06:46
@atsju
Copy link
Collaborator Author

atsju commented May 17, 2025

OK I have finally been able to run Valgrind. There is a leak but not here.

there:

astigPolargraph * graph = new astigPolargraph( list);

No parent is passed, so this never gets deleted.

Also, during my test I saw that you cannot close DFTFringe as long as the astigPolarGraph window is open. In fact you can not do anything in DFTFringe with this window open. Is this expected behavior ?

qtHaveModule(printsupport): QT += printsupport

QMAKE_CXXFLAGS += -std=c++11
QMAKE_CXXFLAGS += -std=c++14
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg! I tried to do this change a year ago to fix a bug releated to unicode characters in file names. I struggled with this and I couldn't get dftfringe to complile. QT did not like this change so I had to change it back. And you just change one character and it compiles fine! Okay this is one thing that sucks about qt creator.

This should fix the unicode character bug in file names issue as Dale's code uses stdlib file functions (fileInputStream or something like that).

Unfortunately I can't do this same change in qt creator unless there is a newer version of it that supports this.

I'm going to start the release process before this merge because I'm worried about what else might break when changing to a newer compiler and I wasn't planning on doing another multi-hour QA session.

@gr5
Copy link
Collaborator

gr5 commented May 17, 2025

I tagged master and started the build process so this can be merged now if you want. I reviewed the changes so far and I approve but I didn't click the review button as I'm not sure if you have more warnings to fix. Anyway there is no rush as these won't make today's release.

@atsju
Copy link
Collaborator Author

atsju commented May 17, 2025

about C++14 this is linked to compiler. If you download the compiler with aqt and link it into any old QTcreator it should work. I build with QTcreator on windows too.

Maybe Dale could update his build setting to use DFTFringe.pro instead DFTFringeDale.pro ? I know you have your build setup on your computer but now that I have the whole process explained in readme you should be able to switch. It would limit discrepancies between local and remote build.

@atsju atsju marked this pull request as ready for review May 18, 2025 16:13
@atsju atsju merged commit 76c5fc2 into githubdoe:master May 18, 2025
8 checks passed
@atsju atsju deleted the JST/warning branch May 18, 2025 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants