Context Menu operations on Folder as Workspace tree #556

SimLV · 2024-05-07T18:06:01Z

For now:

New Folder (for folders and root)
Delete
Rename
Save here (save active document under a new name)

dail8859

Appreciate the PR! I knew this functionality was definitely needed. I think this is a great first start. I'm sure there will be more added to the menu eventually.

dail8859 · 2024-05-08T01:36:42Z

src/NotepadNext/docks/FolderAsWorkspaceDock.cpp

+    ui = new Ui::FolderAsWorkspaceDock;
+    model = new QFileSystemModel(this);


Any particular reason these couldn't be kept in the initializer list?

There was warning about -Wreorder I don't get how to remove

dail8859 · 2024-05-08T01:39:03Z

src/NotepadNext/docks/FolderAsWorkspaceDock.cpp

 #include "ui_FolderAsWorkspaceDock.h"

-#include <QFileSystemModel>
+QString NEW_DIR_TEMPLATE("dir_%1");


I'd have to look deeper how this is used but might make sense to keep it inside the class so this can be translated.

dail8859 · 2024-05-08T01:43:29Z

src/NotepadNext/docks/FolderAsWorkspaceDock.cpp

+    auto doc = window->currentEditor();
+    QString dstName(parentDir.absoluteFilePath(doc->getName()));
+
+    if (doc->saveAs(dstName) != QFileDevice::NoError)


In most cases I prefer the "happy path" to be the first part (e.g. switching this to ==). Just helps with readability since you have to skip over the failure case and look at the else clause to see the natural flow.

Also, this is a bit of a code 'smell' that the FolderAsWorkspace is messing with the lower editor API. Might be possible to use something like window->saveFileAs(...) so that incase there are errors it will be handled the same way as a normal file would if it failed.

I separate UI questions inside MainWindow and use them here instead.

dail8859 · 2024-05-08T01:45:15Z

src/NotepadNext/docks/FolderAsWorkspaceDock.cpp

+void FolderAsWorkspaceDock::on_actionSaveHere_triggered()
+{
+    QDir parentDir(model->filePath(lastSelectedItem));
+    auto doc = window->currentEditor();


To keep consistent with the rest of the code base, rename doc to editor.

dail8859 · 2024-05-08T01:46:00Z

src/NotepadNext/docks/FolderAsWorkspaceDock.cpp

+    QString dstName = NEW_DIR_TEMPLATE.arg(i);
+
+    auto newItem = model->mkdir(lastSelectedItem, dstName);
+    if (!newItem.isValid()) {


Switch this condition as well.

dail8859 · 2024-05-08T01:46:58Z

src/NotepadNext/docks/FolderAsWorkspaceDock.cpp

+{
+    QDir dir(path);
+    QString fileName = dir.absoluteFilePath(oldName);
+    for(auto &&editor : window->editors())


Might also leverage something in window...might need something new like renameFile to take an editor.

There is some mess between MainWindow <-> EditorManager and also ScintillaNext as a view vs some nonexistent Content which could be file or i.e. temporary buffer

Moved parts into MainWindow

dail8859 · 2024-05-08T01:47:44Z

src/NotepadNext/docks/FolderAsWorkspaceDock.cpp

+void FolderAsWorkspaceDock::on_actionDelete_triggered()
+{
+    QString path(model->filePath(lastSelectedItem));
+    QMessageBox::StandardButton reply = QMessageBox::question(this, tr("Delete Item"),


Probably use window->moveFileToTrash() here.

Implemented both options but I have no windows box nearby to test on windows.

dail8859 · 2024-05-08T01:48:34Z

src/NotepadNext/docks/FolderAsWorkspaceDock.h

+    void on_actionSaveHere_triggered();
+    void on_actionNewFolder_triggered();
+    void on_actionRename_triggered();
+    void on_actionDelete_triggered();


Qt doesn't recommend the auto connect slots and I haven't used them elsewhere throughout the codebase, but for now it is fine. It can be cleaned up later.

M.. why/where? connectSlotsByName has no such notices at any version.

clazy is reporting it:

https://github.com/KDE/clazy/blob/1.11/docs/checks/README-connect-by-name.md

It is good scripting usage to automate creation of all these boilerplate as a lua script inside NotepadNext.
For some future version obviously

local luaxml = require 'LuaXML' local file_name = nn.activeEditor.path:gsub('.ui$') local cpp_editor = nn.findEditorByPath(file_name .. '.cpp') local h_editor = nn.findEditorByPath(file_name .. '.h') assert(cpp_editor and h_editor, 'Please open corresponding .cpp and .h') local cpp_bookmark = cpp_editor:findNamedBookmark('ctor signals') local h_bookmark = cpp_editor:findNamedBookmark('slots') assert(cpp_bookmark and h_bookmark, 'Please set "ctor signals" and "slots" bookmarks') local items = {} local tree = xml.load(file_name .. '.ui') tree:iterate(function(el) table.insert(items, el.name) end, 'action', nil, nil, true) local class_name = find_class_name(cpp_bookmark) -- TODO for i,name in ipairs(items) do local method_name = 'on'.. name:gsub('^[a-z]', function(s) return s:upper() end) .. 'Triggered' local full_method = 'void '..method_name..'()' if not h_editor:findText(full_method) then h_bookmark:appendText(full_method .. ';\n'); cpp_bookmark:append('connect(ui->'..name..', &QAction::triggered, this, &'.. class_name.. '::'..method_name..');') cpp_editor:append(full_method..'\n{\n}\n') end end

Interesting idea, though I don't see Lua as a long-term solution. It was originally just added for debugging/testing purposes

I see lua as very good "Glue language" to customize and script stuff. I am going to use NotepadNext as very customizable light editor (instead of NotepadQQ and ZeroBraneStudio) on projects where I don't need big IDE like PyCharm

dail8859 · 2024-05-08T01:50:10Z

src/NotepadNext/docks/FolderAsWorkspaceDock.ui

@@ -27,19 +27,77 @@
    <property name="bottomMargin">
     <number>0</number>
    </property>
+    <item>
+     <widget class="QMenu" name="menuFile">


Can QtCreator create a QMenu in the ui file? or some other method (e.g. manually)? I'm just interested since I've never seen this done before :)

It can't. I googled for it but at least Qt 5 can't. For me it looks like minor inconvenience everyone accustomed to.

dail8859 · 2024-05-08T21:17:51Z

I'll take a crack at this and push any updates. I think I might have been confused a bit and told you wrong in places. I guess I was thinking the "File List" window...which means there was always an editor object associated with it (and thus easier to use the window). But realize now for the Folder as Workspace it is more direct file system modification potentially.

dail8859 · 2024-05-09T03:00:30Z

I pushed updates to this branch. I made a quick first round of cleanups...hopefully I didnt break anything that was working previously.

Some things to note:

I removed the permeant deletion for now. Think its better to just give the user an option move it to the trash. (if someone really wants it maybe it is a configuration later)
I understand the intent of closeByPath()...the difficult part is trying to half use the FaW as an explorer window that also keeps the opened files in sync (renamed, deleted, etc)...which is part of the reason why I didn't tackle this in the first place :) Notepad++ doesn't provide those file level operations so it never has to worry about that. For example:
It might be better to leverage the EditorManager to future proof some things when finding an editor by the file path, because in the future if there are multipe windows it is resposinble for coordinating the editors between them. Plus if an editor is opened multiple times etc.

Sorry...late night brain dump :)

SimLV · 2024-05-09T05:05:37Z

src/NotepadNext/dialogs/MainWindow.cpp

@@ -1369,7 +1356,10 @@ void MainWindow::moveFileToTrash(ScintillaNext *editor)

    if (askMoveToTrash(filePath)) {
        if (editor->moveToTrash()) {
-            closeByPath(filePath);
+            closeCurrentFile();


Why are you closing current file? One could open one file and delete another one.

You are right! Good catch! 😄 This is what I get for trying to modify it on little sleep.

SimLV · 2024-05-09T05:08:37Z

I was searching for permanent deletion solution like in file explorers (Option+Delete means permanent delete) but this needs subclassing of QTreeView.
"Eat your own dogfood". At least minor file manipulations (new/rename/delete) are useful for me.
I am working+thinking on "Run..." stuff but this is more complex

Do you need some other rework for this branch from me for now?

dail8859 · 2024-05-09T11:47:08Z

I was searching for permanent deletion solution like in file explorers (Option+Delete means permanent delete) but this needs subclassing of QTreeView.

Yeah I think if something isn't obvious it is good to err on the side of caution. The user needs to accept the prompt anyways so best to move it to the trash. Again this could be an app setting or something in the future but unless there is a real "need" I don't think it is needed. If you need to delete something "permanently" I think it would be best not to use a text editor's file management window :)

"Eat your own dogfood". At least minor file manipulations (new/rename/delete) are useful for me.

Agreed. They are very nice and normally 'expected' features if you are going to be showing a list of files.

I am working+thinking on "Run..." stuff but this is more complex

I'm not saying this is needed for now. I was just showing an example of what Notepad++ does. This stuff can always be added in later if there is a real use-case for it.

Do you need some other rework for this branch from me for now?

No, I don't believe so currently. I need to find some more time to just sit down with it and work through some of this.

dail8859 · 2024-05-10T02:00:28Z

So I had some time to step back and think about this. There is definitely more complexity that has to be handled to implement it correctly.

If you rename a folder, now you have to check every file in that folder as a possible candidate that is opened for editing because it's file path could have changed.

Same with deleting a folder. You now have to check every file.

I know the application doesn't gracefully handle when files change outside of the application yet, but there could be race conditions that the application sees the file change on disk and notify the user the file is missing/edited before the FaW logic could tell the application it is gone/edited.

Also there's a case that you delete the file, but the file changes weren't saved, then I guess you just close the file anyways? (not looking for a direct answer, just thinking out loud).

Most of these cases are possible to handle but could run into cases that a folder could contain hundreds or thousands of files and make the entire situation worse.

Remembering now, all this was why long ago I never added a context menu since it is desirable to have these kinds of file operations but hard to handle gracefully.

SimLV · 2024-05-10T15:06:06Z

Some of such stuff could be fixed using timer and check for all opened files once a second.
Going to fix:

Check files on Rename folder
- all Editors will be renamed
Check files on Delete folder
- saved Editors will be closed
- unsaved Editors will be kept open

dail8859 · 2024-05-10T17:36:54Z

Some of such stuff could be fixed using timer and check for all opened files once a second.

Honestly that definitely doesn't not sound like a good idea and timers are usually just band aid for race conditions.

SimLV · 2024-05-10T19:16:58Z

Then QFileSystemWatcher for each Editor bound to a file

+ getPath dropped from ScintillaNext + getFilePath now uses Qt directory separators

SimLV · 2024-05-12T07:31:20Z

@dail8859 could you please check and possible merge this PR.
I think further improvements could be in other PRs.

dail8859 · 2024-05-12T15:19:17Z

I want to find some more time to review this again but not sure when that will be yet to identify any additional issues that need addressed or noted for future PRs.

I'll be the first to admit I'm quite particular about code that gets merged and want to make sure. I do appreciate the time and effort you and others contribute so I'll do my best to set aside some time for this.

dail8859 · 2024-07-08T21:41:01Z

I did pick this back up recently and started hacking away at it. In the end I might simplify it down for now to get alot of the changes merged in and can tackle some of the technically challenging functionality later to narrow down testing it.

…rkspace_operations

SimLV · 2024-09-14T07:43:03Z

Merged current master and tested

SimLV added 4 commits May 7, 2024 19:23

Workspace operations: Rename

882066c

Workspace operations: Delete, New Folder, Save Here

08a04c9

Rename editor after renaming a file

45fcfcb

'Delete' removes folders correctly

fe945df

SimLV force-pushed the workspace_operations branch from ad5b0fb to fe945df Compare May 7, 2024 19:48

dail8859 requested changes May 8, 2024

View reviewed changes

Improvements after review

58dbd4c

SimLV requested a review from dail8859 May 8, 2024 18:04

newDirTemplate as FolderAsWorkspaceDock.cpp

135b880

Cleanup round 1

799ad73

SimLV commented May 9, 2024

View reviewed changes

SimLV added 2 commits May 10, 2024 22:57

Rename at Folder as Workspace View should update all opened Editors

c936ab4

+ getPath dropped from ScintillaNext + getFilePath now uses Qt directory separators

Delete at Folder as Workspace View should work now.

5885f46

SimLV mentioned this pull request Jun 5, 2024

Support for Sessions #554

Open

Agrament approved these changes Aug 10, 2024

View reviewed changes

Merge branch 'master' of [email protected]:SimLV/NotepadNext.git into wo…

0ed5f25

…rkspace_operations

		ui = new Ui::FolderAsWorkspaceDock;
		model = new QFileSystemModel(this);

Context Menu operations on Folder as Workspace tree #556

Are you sure you want to change the base?

Context Menu operations on Folder as Workspace tree #556

Conversation

SimLV commented May 7, 2024

dail8859 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimLV May 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dail8859 commented May 8, 2024 • edited Loading

dail8859 commented May 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimLV commented May 9, 2024

dail8859 commented May 9, 2024

dail8859 commented May 10, 2024

SimLV commented May 10, 2024 • edited Loading

dail8859 commented May 10, 2024

SimLV commented May 10, 2024

SimLV commented May 12, 2024

dail8859 commented May 12, 2024

dail8859 commented Jul 8, 2024

SimLV commented Sep 14, 2024

SimLV May 9, 2024 •

edited

Loading

dail8859 commented May 8, 2024 •

edited

Loading

SimLV commented May 10, 2024 •

edited

Loading