Skip to content

Add the feature to merge projects#122

Merged
sanny32 merged 1 commit into
sanny32:devfrom
raspopov:i121-merge-projects
Jul 13, 2026
Merged

Add the feature to merge projects#122
sanny32 merged 1 commit into
sanny32:devfrom
raspopov:i121-merge-projects

Conversation

@raspopov

@raspopov raspopov commented Jul 4, 2026

Copy link
Copy Markdown
Contributor
  • 💡 Added the feature to select multiple files when opening. The first file becomes the main project, and the others are merged into it.
  • 💡 Added the feature to drag and drop project files into the program window, with a prompt to merge them.
  • 🐞 Fixed a bug where a modified project was overwritten without a save prompt when opening a new file in MainWindow::on_actionOpenProject_triggered().
  • 🐞 Fixed unnecessary project modifications when any window is normally activated in MainWindow::on_mdiSubWindowActivated().

Changes to the codebase:

  • Added the AppProject::filePath() method, which returns the file path to the project, replacing the removed MainWindow::_projectFilePath class member that duplicated this functionality.
  • All functionality related to closing a project has been moved from the AppProject::loadProject() method to the AppProject::closeProject() method.
  • The AppProject::saveProject() method now remembers the save directory on its own; previously, all calls to saveProject() were paired with setSavePath().
  • Added the MainWindow::closeProject() method—removed duplicate code for closing the project and prompting to save changes.
  • The MainWindow::dragEnterEvent() and MainWindow::dropEvent() methods have been added to implement file dragging.
  • The signature of the MainWindow::loadProject() function has been extended to support adding multiple files to a single project and handling user-initiated cancellation of the operation.
  • The MainWindow::saveProject() method now maintains its own list of recent files using addRecentProject().

Fixes #121.

@sanny32 sanny32 self-requested a review July 7, 2026 05:42

@sanny32 sanny32 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In AppProject::loadProject(), every loaded file ends with _mainWindow->applyConnections(defs, conns) at src/appproject.cpp:1968, and applyConnections() does _mbMultiServer.setModbusDefinitions(defs) plus replaces the connect menu with only conns at src/mainwindow.cpp:1675.
So when MainWindow::loadProject(filename, replace=false) is used for the second/next project, forms are appended, but Modbus definitions and saved connections are overwritten by the last merged file. That makes “merge projects” lossy and order-dependent.

@raspopov raspopov force-pushed the i121-merge-projects branch from c65d09c to f42e183 Compare July 8, 2026 15:26
@raspopov

raspopov commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

That makes “merge projects” lossy and order-dependent.

Thank you, that's a correct point. 👍 I've made the necessary changes to the patch; now the register descriptions are merged correctly, and project-wide settings are ignored for subsequent files being merged.

@raspopov raspopov requested a review from sanny32 July 8, 2026 17:38

@sanny32 sanny32 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. This is definitely safer now: the merged project no longer overwrites the current project's ModbusDefinitions and connection list, which addresses the main data-loss concern I had.

I still think the merge semantics should be clarified before merging. As currently implemented, additional projects contribute forms and address-space data, but their ModbusDefinitions, connections, global view settings, tab order, and active-window state are ignored. That may be a valid design, but then the UI/PR description should make it clear that this is more like importing forms/data into the current project rather than a full project merge.

A couple of smaller code issues remain:

  • MainWindow::dragEnterEvent() and MainWindow::dropEvent() are missing the required Doxygen blocks (\brief, and \param where applicable).
  • Drag-and-drop currently accepts any existing file, not just .omsim files. Since AppProject::loadProject() returns void, MainWindow::loadProject() cannot tell whether loading actually succeeded, so dropping an invalid file may result in confusing behavior.

@raspopov raspopov force-pushed the i121-merge-projects branch from f42e183 to 5eb23fc Compare July 11, 2026 09:58
@raspopov

raspopov commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Added an explanatory label for the user, added documentation comments, and added the ability to select project files from the list of dropped files in the window.

Also added the ability to drag and drop entire directories containing files.

The project has been rebased to the top of the dev branch.

P.S. The dev branch has compilation error on MSVC2022 + Qt 5.15.2:

Building CXX object CMakeFiles\omodsim_tests_modbusmessage.dir\tests\test_modbusmessage.cpp.obj
  FAILED: CMakeFiles/omodsim_tests_modbusmessage.dir/tests/test_modbusmessage.cpp.obj 

C:\Qt\5.15.2\msvc2022_64\include\QtTest\qtestcase.h(352): error C2593: 'operator ==' is ambiguous
  C:\Qt\5.15.2\msvc2022_64\include\QtCore/qstring.h(806): note: could be 'bool operator ==(const QString &,const QString &) noexcept'
  C:\Qt\5.15.2\msvc2022_64\include\QtTest/qtestcase.h(352): note: or       'built-in C++ operator==(int, int)'
  C:\Qt\5.15.2\msvc2022_64\include\QtTest/qtestcase.h(352): note: while trying to match the argument list '(const T, const T)'
	  with
	  [
		  T=ModbusException
	  ]
  C:\Qt\5.15.2\msvc2022_64\include\QtTest/qtestcase.h(352): note: the template instantiation context (the oldest one first) is
  C:\Projects\Modbus\OpenModSim\src\tests\test_modbusmessage.cpp(132): note: see reference to function template instantiation 'bool QTest::qCompare<ModbusException>(const T &,const T &,const char *,const char *,const char *,int)' being compiled
	  with
	  [
		  T=ModbusException
	  ]

@raspopov raspopov requested a review from sanny32 July 11, 2026 10:02
@sanny32 sanny32 merged commit 060b95e into sanny32:dev Jul 13, 2026
4 checks passed
@raspopov raspopov deleted the i121-merge-projects branch July 13, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants