[Windows] Fix the "Save SSH connections" functionality

[alacasta]

I have started investigating it. What i saw is that when creating the SSHConnectionSerializer object in the main screen, the file savedConnectionsFilePath + "/qjournalctl" does not exist and then, at sshconnectionserializer.cpp:20, the code continues (computes the return path)

    // File IO or exit
    modifiedConnectionsFile = false;
    if(!savedConnectionsFile.exists()) {
        return;
    }

When creating the file by clicking on the save button, the file is neither created. Moreover, I get strage characters

Can you explain the supossed flow of how the save operation ends into serializing such information into savedConnectionsFilePath + "/qjournalctl?

[pentix]

Thanks for having a look at it. The functions used from the QStandardPaths module are supposed to return the path to the (persistent) configuration directory that the application is allowed to write to.

The idea is to save the SSH connections into a file called qjournalctl inside this directory (therefore adding the filename to the retrieved file path)

We'll have to check the Qt Docs for the proper usage / error handling on Windows. The "encoding error" seems strange, that's probably related to the fact the file can't be opened properly...

[pentix]

Indeed, QStandardPaths returned the path delimited by / where Windows generall prefers \, so I used a function from QDir to convert them appropriately:

https://github.com/pentix/qjournalctl/blob/35cb4ee33ae7883425ac067bcdcc420acf3a162b/src/sshconnectionserializer.cpp#L20

Also, the settings might be stored in application specific folders that do not exist when qjournalctl is used for the first time. In that case, we'll just create the directory:

https://github.com/pentix/qjournalctl/blob/35cb4ee33ae7883425ac067bcdcc420acf3a162b/src/sshconnectionserializer.cpp#L25

This seems to already suffice to fix the issue; Are you still able to reproduce the weird connection names? 🤔

[alacasta]

Again and as i asked you yesterday, it is not clear the flow.

Before the latest change, i suppossed that the file should be at C:\Users\Asier\AppData\Local\qjournalctl although with your latest change, what's the filename? You have now created a folder C:\Users\Asier\AppData\Local\qjournalctl\ but the variable

    savedConnectionsFile.setFileName(savedConnectionsFilePath + "qjournalctl");

Is not correctly setting its value

[alacasta]

It is still throwing wrong encoding....

[alacasta]

Again,

In which point is theoretically created the file savedConnectionsFile?

[alacasta]

Again,

In which point is theoretically created the file savedConnectionsFile?

Okay, the file is correctly saved when the application is destroyed (at `SSHConnectionSerializer::~SSHConnectionSerializer()). however, the encoding is still wrong

[pentix]

Sorry if I haven't answered your questions:

Again and as i asked you yesterday, it is not clear the flow.

Before the latest change, i suppossed that the file should be at C:\Users\Asier\AppData\Local\qjournalctl although with your latest change, what's the filename?

The flow of the constructor is as follows:

1. QStandardPaths::writableLocation(QStandardPaths::ConfigLocation) returns the folder where we want to save the the settings.
2. (With the latest change) this folder is created if it didn't exist prior.
3. savedConnectionsFile which is an instanciated QFile set to point to a file called qjournalctl inside the configuration folder we retrieved in 1.
4. If this file does not exist, there are no settings that we can read. (Nothing has ever been saved) We are therefore at the end of the constructor.
5. If this file does exist, we open it for reading, read the contents and let the QJson parse the contents.
6. We add those parsed settings to a local representation (inside the serializer), that can be used to easily access the hosts, etc.

The flow of the destructor is as follows:

1. If the local representation has been modified (i.e. hosts added, changed, etc.) we will have to save the local representation to disk.
2. We retrieve a json encoded copy of the settings.
3. Now we open (and implicitly create) the file you mentioned: https://github.com/pentix/qjournalctl/blob/35cb4ee33ae7883425ac067bcdcc420acf3a162b/src/sshconnectionserializer.cpp#L69
4. We write the JSON representation of the settings into the file here: https://github.com/pentix/qjournalctl/blob/35cb4ee33ae7883425ac067bcdcc420acf3a162b/src/sshconnectionserializer.cpp#L70
5. And by closing the file we're finished with the destructor.

Is this answering your question?

Again,

In which point is theoretically created the file savedConnectionsFile?

The instance of QFile is itself just a member of the SSHSettingsSerializer but we set the path and everything in the constructor and open the file before reading and writing changes to it. So the file on the harddisk is created when opening it in TRUNCATE mode and writing to it (3., 4.)

[pentix]

Before the latest change, i suppossed that the file should be at C:\Users\Asier\AppData\Local\qjournalctl although with your latest change, what's the filename?

The path of the folder should be C:\Users\Asier\AppData\Local\qjournalctl, which results in the full path of the file being C:\Users\Asier\AppData\Local\qjournalctl\qjournalctl

You have now created a folder C:\Users\Asier\AppData\Local\qjournalctl\ but the variable

    savedConnectionsFile.setFileName(savedConnectionsFilePath + "qjournalctl");

Is not correctly setting its value

Hmm that is strange, printing savedConnectionsFile.filePath() works for me, I think the "not accessible" might be due to the debugger limitations / compiler optimizations?

[pentix]

Since this still happens

It is still throwing wrong encoding....

Could open the settings file at C:\Users\Asier\AppData\Local\qjournalctl\qjournalctl manually and check it's contents?

Thank you very much, I hope this helps

[alacasta]

Before the latest change, i suppossed that the file should be at C:\Users\Asier\AppData\Local\qjournalctl although with your latest change, what's the filename?

The path of the folder should be C:\Users\Asier\AppData\Local\qjournalctl, which results in the full path of the file being C:\Users\Asier\AppData\Local\qjournalctl\qjournalctl

You have now created a folder C:\Users\Asier\AppData\Local\qjournalctl\ but the variable

    savedConnectionsFile.setFileName(savedConnectionsFilePath + "qjournalctl");

Is not correctly setting its value

Hmm that is strange, printing savedConnectionsFile.filePath() works for me, I think the "not accessible" might be due to the debugger limitations / compiler optimizations?

That screenshot corresponded to when i didn't realize the flow; the file was not created so then, it was right. 👍

This is clear now!

[alacasta]

Since this still happens

It is still throwing wrong encoding....

Could open the settings file at C:\Users\Asier\AppData\Local\qjournalctl\qjournalctl manually and check it's contents?

Thank you very much, I hope this helps

This is the content. Seems that the problem comes from the translation from the textboxes to the SSHSerializer object.

[alacasta]

The problem comes from the conversion at

const char *SSHConnectionSettings::qstringToChar(QString s)

@pentix I'm working on it 👍

[alacasta]

I think i got it! https://github.com/pentix/qjournalctl/pull/54

🎉