Fixes bug #439

[anubh4v]

Issue was with the do while loop in line 123 of fileformatdialog.cpp. Condition was true instead of false.

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

[welcome]

Thanks for opening this pull request! Please check out our contributing guidelines and check for the automated tests.

[anubh4v]

@cryos please look into this.

[ghutchis]

I'm not sure I understand. One commit changes a line - and the next commit changes it back?

[anubh4v]

@ghutchis first commit deletes "while(true)" and the second commit adds "while(false)".

[ghutchis]

Sorry for the delayed review. I think I'd be happier if QString fileName was moved out of the loop and the termination would be:

  } while (fileName.isEmpty());

It's functionally the same, but I think the code would be more readable.

[anubh4v]

If QString fileName is moved outside the loop then we won't be able to change the format when continue is encountered in the loop. Since you need to change the fileName extension to change the format.

[ghutchis]

I mean something like this:

  QString fileName;
  do { // jump point for continue statements on retry
    fileName = QFileDialog::getSaveFileName(parentWidget, caption, dir, realFilter);

    if (fileName.isEmpty()) // user cancel
      return result;
.. .
  } while (fileName.isEmpty());

[ghutchis]

Thanks - just waiting for Mac / Win builds to complete (which they should)

[ghutchis]

Sorry, one last minor thing before merge - can you sign off on your commits?
(This indicates that these changes are yours.)
https://github.com/OpenChemistry/avogadrolibs/pull/517/checks?check_run_id=2253882029

[welcome]

Congrats on merging your first pull request! 🎉 Thanks for making Avogadro better for everyone!

[ghutchis]

Thanks for the fix and your patience on this one. I have more flexible time to contribute to Avogadro for the next few months, so future pull requests should get merged faster.