New file feature

[franciscomartinez45]

A new file can only be created if the text editor is not empty and the file has been saved, if any changes to an existing file are made and addressed accordingly, or if the text editor is empty and there is no saved file.

[chrisdedman]

Hi @franciscomartinez45

Thank you for your time and contribution! I have a couple of requests for changes (please see the comments I’ve provided) as well as a logic question regarding the test files.

Once these changes are addressed, the PR should be nearly ready to merge. I do have one additional request before merging: could you break down the newFile function slightly to improve modularity? This will improve maintainability and reusability, especially since I’ll need the logic that detects whether a file has been edited to be used in other functions as well.

Thank you again for your hard work on this!

[chrisdedman]

This will again create a new file in a new window, and not in the current one.
You should not create a new window object as an object is already created at the initialization of the software (the m_mainWindow, the m_ is how you know it is available as a member here).
Plus, you also want to reset the editor (with member editor object m_editor).

Finally, you also want this to be outside of the condition (outside the if/else statement). Otherwise, it will not be applied properly once you fix what I said above.

Here is a possible implementation:

    // Clear the editor and reset the state for a new file
    setCurrentFileName("");
    m_editor->clear();
    if (m_mainWindow)
    {
        m_mainWindow->setWindowTitle("CodeAstra ~ untitled");
    }

[franciscomartinez45]

I have made the fixes so that the editor is only reset. The reason I have created an else statement for cleaning the editor is that if the user exits during the save process, then the user still has the current state of the editor intact. The editor will only be reset to an untitled file if there are no changes to the current saved file or if the file has not been saved. Please let me know if you would like me to keep my logic in there for detecting changes to the current file.

[chrisdedman]

Oh, I see @franciscomartinez45 . Yeah. The problem is that when you save the changes, it does not create a new file (I tested it right now). You may want to wrap this inside an if statement instead of an else statement, such as:

    if( !m_currentFileName.isEmpty())
    {
        setCurrentFileName("");
        m_editor->clear();
        m_mainWindow->setWindowTitle("Code Astra ~ untitled");
    }

Here, if the user is changing their mind, the code will check if the file name is empty; if not empty, it will go through the process and clear the editor. If it is empty, that would mean that the file has not been saved, so it won't clear it.

[chrisdedman]

What is the reason for changing QCOMPARE_EQ to QCOMPARE_H?

The change will introduce a serious issue. From my research, QCOMPARE_H is not a valid test assertion macro and has no effect in testing. The original implementation with QCOMPARE_EQ is correct as it checks the equality of the menu bar’s action count and provides meaningful test output in case of failure.

[chrisdedman]

Same question as the other test file. I am not sure to understand your logic in changing QCOMPARE_EQ to QCOMPARE_H. I will wait for your answer, but I would prefer to change back to the original implementation.

[franciscomartinez45]

Thanks for all the feed back Chris! I would like to note that I am working on WSL. When I was compiling the code I kept getting issue with the QCOMPARE_EQ function. Instead, it advised me to use QCOMPARE_H. No real logic behind using it besides the fact it makes my code work on my machine. I would also like to not that QCOMPARE() also works. Not sure if it is a machine independent issue.

[chrisdedman]

hi @franciscomartinez45 . Ok, so I did some research, and your Qt6 version on WSL is probably not updated. QCOMPARE_EQ() is a better and safer feature for testing than QCOMPARE() and was introduced in Qt 6.6.0 (see here https://doc.qt.io/qt-6/qtest.html#QCOMPARE_EQ).

Could you check your Qt version by adding a debugging line (maybe at the start of the program in the main function) and show me what you get?

qDebug() << QT_VERSION_STR;

If your version is greater than 6.6.0, then it would be best to update with QCOMPARE() for now, as QCOMPARE_H() isn't intended for this purpose

[chrisdedman]

Thank you for the update. Other than the little update for this feature and the problem with the Test, everything looks good. Once you update this, I can merge it.

[chrisdedman]

As mentioned in the comment in the conversation, this would be better in that case (please, see the latest comment I left after you updated from my previous review):

    if( !m_currentFileName.isEmpty())
    {
        setCurrentFileName("");
        m_editor->clear();
        m_mainWindow->setWindowTitle("Code Astra ~ untitled");
    }

[chrisdedman]

See comment left on the previous conversation on this file. Thanks

[chrisdedman]

Hi @franciscomartinez45 . First of all, I want to thank you for your contribution. I also want to let you know that over the weekend, I will be adding the 2 changes I left in the PR comment and merging your contribution. The comments are not that big, but if I do that before you have time to do it, please take a quick look at what I said. Does not add much value in learning, but good information :). Once again, thank you for taking the time to contribute, and I hope to see a new PR from you in the future :).

[chrisdedman]

LGTM! 🚀