Set example file, improved template text, use namefilter for *.xml

[stefonarch]

At least 2 things missing I've no clue how to achieve:

• set filedialog path to ~/.config/qterminal.org/ instead of $HOME
• save qterminal_bookmarks_example.xml in the config location at first run

As it is it will show the small example syntax when qterminal_bookmarks_example.xml is not present.
With the above it would close #886 IMHO

[tsujan]

At least 2 things missing I've no clue how to achieve

I'll look into them but first should find the best logic for dealing with the example file — have forgotten the code because of wrestling with other codes ;)

[stefonarch]

Take your time :)

[tsujan]

The above two lines should be:

    dia.setFileMode(QFileDialog::ExistingFile);
    dia.setNameFilter(tr("XML files (*.xml)"));

But please read my next comment before changing them.

[tsujan]

IMO, saving qterminal_bookmarks_example.xml to the config location at first run is unusual as well as problemtic, This is my plan:

• Select the current bookmarks file if it exists.
• If the user has no bookmarks fiile yet but there's one in QTerminal's data directory, open that directory.

To do it, I need to make another PR that contains yours and supersedes it.

What do you think?

[tsujan]

BTW, the title of the dialog, namely "Open or create bookmarks file", doesn't make sense to me because that dialog only opens a file. It should be "Open bookmarks file" and dia.setOption(QFileDialog::DontConfirmOverwrite, true); is redundant.

[tsujan]

Again, I encountered strange things in QTerminal's GUI ;) What if the user types the path of a nonexistent XML file inside the entry of the "Bookmarks" page of QTerminal's preferences dialog? Where is the confirmation dialog when the user edits a bookmark file from inside it? To say nothing of how the user can create a bookmarks file with it (he can't).

[stefonarch]

• Select the current bookmarks file if it exists.

• If the user has no bookmarks fiile yet but there's one in QTerminal's data directory, open that directory.

To do it, I need to make another PR that contains yours and supersedes it.

What do you think?

The existing bookmarks file will be already set in existing .ini file I think. And opening the example file in data directory will be read-only, and saving it in the config location is not intuitive. Maybe a button "use example file"? No clear idea atm.

BTW, the title of the dialog, namely "Open or create bookmarks file", doesn't make sense to me because that dialog only opens a file. It should be "Open bookmarks file" and dia.setOption(QFileDialog::DontConfirmOverwrite, true); is redundant.

This is done, but without DontConfirmOverwrite, true it will ask when saving changes?

 -   QFileDialog dia(this, tr("Open or create bookmarks file"));
 + QFileDialog dia(this, tr("Select bookmarks file"));

[tsujan]

And opening the example file in data directory will be read-only, and saving it in the config location is not intuitive.

QTerminal's preferences dialog could save it to the config folder if its Apply or OK button is pressed.

This is done, but without DontConfirmOverwrite, true it will ask when saving changes?

That file dialog only opens an XML file. It isn't responsible for saving. Saving will be done if the Apply or OK button of QTerminal's preferences dialog is pressed and there is a file to save to.

Nothing is intuitive in this section of preferences dialog.

[tsujan]

and saving it in the config location is not intuitive.

But you yourself proposed that we save it to the config location at first run. What I'm saying is saving it only if the user explicitly selects it and confirms the preferences dialog.

[tsujan]

Yet another method is as follows:

We could remove qterminal_bookmarks_example.xml and put its content inside the view of the Bookmarks page, instead of the current poor XML text.

[stefonarch]

I was thinking about this too, but it's ugly to set all those spaces and \ needed.

What I'm saying is saving it only if the user explicitly selects it and confirms the preferences dialog.

The actual "ok" button isn't quite clear for this, but this would be the easiest way to do. Having 2 examples files isn't smart.

[tsujan]

The actual "ok" button isn't quite clear for this

If you mean the OK button of QTerminal's dialog, I agree, but, as you said, it's the easiest way. Moreover, as I mentioned above, there should be a confirmation dialog: the app shouldn't overwrite a file without asking the user.

[stefonarch]

What if the user types the path of a nonexistent XML file inside the entry of the "Bookmarks" page of QTerminal's preferences dialog?

Maybe this lineedit should be removed or showing location without editing.

[tsujan]

Maybe this lineedit should be removed or showing location without editing.

The line-edit will be useful if we enhance the functionality of the Bookmarks page: the user could type a path and expect that it'll be created on clicking the OK/Apply button of QTerminal's dialog.

EDIT: I'm so confused by this GUI that I make several typos in my comments ;)

[stefonarch]

But it's already clear (if "search..." button opens config directory) where he has saved it, actually the button should simply open the location of the file wherever it is.

EDIT: I'm so confused by this GUI that I make several typos in my comments ;)

same here, but also the ideas... It remembers me my first approach years ago...

EDIT: and not "search..." but "open"

[tsujan]

But it's already clear (if "search..." button opens config directory) where he has saved it,

My point is that the user may type a path, instead of searching for an existing file. That'll be good if the dialog does something with that path, i.e., if it creates it.

I need to think more about the design. Hopefully, I'll find an acceptable solution and will discuss it with you.

[stefonarch]

Path typing can be a source of errors, while clicking through folders in the filedialog is always ok.

[tsujan]

Path typing can be a source of errors, while clicking through folders in the filedialog is always ok.

Yes, but it's a good option for those who have already copied a path to the clipboard or want a path to be created. It's usual to have this functionality in an app.

[tsujan]

My sketch:

1. Change the default XML text to something really usable but keep it very simple.
2. If the user hasn't selected a bookmarks file, show him our example file when he clicks the "Find..." button. If he selects it and confirms QTerminal's preferences dialog, save that file to the config location.
3. If the user has already selected a bookmarks file, show and select it when he clicks the "Find..." button.
4. If the user has typed a nonexistent path, create it if he confirms QTerminal's dialog, and put the usable default XML text (→ 1) into it.
5. Always ask for confirmation when overwriting is needed.
6. Also, show an error dialog if file creation/overwriting is impossible.

[stefonarch]

Looks reasonable.

Change the default XML text to something really usable but keep it very simple.

Imho it should still contain much of bookmarks potentials.

[tsujan]

Imho it should still contain much of bookmarks potentials.

OK, but still as simple as possible. We already have an example file.

Please also note that the above sketch is general. There are several details that I haven't included. Their implementation will take time. The goal is having a Bookmarks page that does what any user may expect from it, without adding a widget to it.

Later, we could discuss more widgets but I think, if the current ones work as expected, the Bookmarks page will be OK.

[tsujan]

Almost done. Just three things:

• I think the overwrite prompt is needed only when the example file is copied to the config directory but a file with the same name already exists (it may happen rarely but is possible). Because, otherwise, when the user clicks Apply or OK in QTerminal's dialog, he explicitly confirms the changes, and we don't need to ask him again.
• I don't think we need to add <?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n<!-- Syntax for bookmarks file-->\n to the beginning of the default XML text. It isn't required and might frighten the user.
• I used this string for the default text:

<qterminal>\n  <group name=\"Change Directory\">\n    <command name=\"Home\" value=\"cd $HOME\"/>\n  </group>\n  <group name=\"File Manager\">\n    <command name=\"Open here\" value=\"xdg-open $(pwd)\"/>\n  </group>\n</qterminal>\n

It's simple and shows the basic functionality.

[stefonarch]

I didn't like much the cd $HOME --it's offending for terminal users who mya know about cd ...

So, when clicking "ok" this will be used or the example file?

[tsujan]

it's offending for terminal users who mya know about cd ...

It just shows the structure; the user could choose the contents. If you have another simple command in mind, please tell me.

So, when clicking "ok" this will be used or the example file?

If the user clicks Apply or OK the first time he runs QTerminal, an XML file with the above contents will be created in his config directory.

If he clicks Find... before clicking Apply/OK at the first run, the app directory will be opened and its example file(s) will be shown. The same thing will happen anytime he enters a nonexistent path in the line-edit or just clears it before clicking Find....

If he chooses an example file from the app directory (the file name isn't important), it'll be copied to his config directory when he clicks Apply/OK (the path will be updated in the line-edit too).

[tsujan]

If you want the example file(s) to be more visible to the user, I could add an extra button with the label "Example", so that it'll open the app directory regardless of what's in the line-edit.

EDIT: In that case, the label "Examples" might be better because you could add more example files.

I mean this:

[stefonarch]

That is quite nice! But not sure about more examples just for the sake of it, in case we need ideas for them ;)

[tsujan]

But not sure about more examples just for the sake of it

Even one file can be considered to show several examples; the plural form "Examples" can be used with one file too. Who knows? You might want to add multiple files later.

[tsujan]

Done in #919