Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any interest in hierarchical Layers support in LibreCAD? Like this #1731

Open
sand1024 opened this issue Jan 29, 2024 · 8 comments
Open

Any interest in hierarchical Layers support in LibreCAD? Like this #1731

sand1024 opened this issue Jan 29, 2024 · 8 comments

Comments

@sand1024
Copy link
Contributor

sand1024 commented Jan 29, 2024

Actually, this is not exactly the issue - currently LibreCAD does not support layers hierarchy and this is not too convenient (yes, of course, Autocad DXF does not support it too).

However, it is possible to emulate hierarchy by using appropriate naming conventions for layers.

So basically I've spent some time and created a widget (similar to Layer List) that does this and also supports a couple of nice operations (like move selection to layer, copy/duplicate layers and so on).

I've uploaded a demo video with screencast that shows the widget on YouTube - https://www.youtube.com/watch?v=kFLRBU7FSCA.
The list of implemented features - in video's description. If you'll see some room for improvements - please let me know.

In general, if such widget is usefull for somebody except me - I could commit into my repo and create merge request ther. If not - well, it will be just my modified copy of LibreCAD.

P.S. not sure whetether the issue there is the right place for discussion of such things - if there is some more suitable place, please let me know.

Thank you!

@dxli
Copy link
Member

dxli commented Jan 30, 2024

This one looks great to me.

@sand1024
Copy link
Contributor Author

Appropriate code is in this repo https://github.com/sand1024/LibreCAD.

All functionality was commited by one large commit - sand1024/LibreCAD@80ed4c2...08dac86

I'm not sure whether it's practical to create pull request at the moment, however, it will be great if somebody may check the implementation and let me know any feedback (both features and implementations).

I think feedback with issues/discussions in that fork's repo will work better rather than directly there? Anyway, I'm more than open for the feedback and suggestions.

@lordofbikes
Copy link
Member

Many thanks for your intention to share this feature!

I've cherry picked your commit 80ed4c2, the fix for rs_actiondefault.h, as this is another issue.
As old as QCAD and unbelievable, that this wasn't found yet 🤣. Nice catch.

I haven't gone through the implementation yet, but I have two recommendations so far.

  • To keep your master in sync with the main repo, you should move the feature commit into a separate branch. This makes pull request and merging much easier. So you should pull the new master with your fix and on this commit (which is different from yours, SHA de9cb37) create a branch for the feature.
    Don't hesitate to ask if you need help with git here.
  • For all the new files please replace the prefix qg_ by lc_. qg_ files are QCAD origins and for newly implemented files we use lc_ to keep them apart.

@sand1024
Copy link
Contributor Author

Thank you for your comments. Yes, basically that issue with define was a bit annoying - I've commiteed initilal version of CMakeLists.txt also (yet without plugins build so far) - hope it may be good start for adding support of cmake for the project too.

BTW, when I've tried to achive build of the project via cmake - I've noticed lots of files that are part of repo but are excluded from build.

It seems that they are not needed so far, so probably it's better to move them in to some separate directory (if they are needed, say, for some historical reasons)? Such files are commented in CMakeLists.txt in that repo.

Regarding the prefix - yes, got your point, I'll refactor naming accordingly. Indeed, it was not clear for me why there are different prefixes for classes, so I've just used conventions from List widget.

If there are naming/style etc. conventions that should be followed to keep the overall codebase of the project - please let me know that.

Also, I think there might be another generic issue with my current implementation - tha generic decomposition and organizing of the code. I'll be glad to know your opinion there.

Actually, I've tried to affect the existing codebase as little as possible, and that's a reason why, for example, dialogs are shown not via QG_DialogFactory or why some operations are implemented not as actions but directly as slotts.

Of course, existing code may be reworked in such way and that may be more consistent approach - however, I'm rather concerned thare that it may introduce some regression issues and will require additional testing.

And indeed, good point regarding git - I'll follow the way you've mentioned, thank you.

@sand1024
Copy link
Contributor Author

commit for renamed version is there sand1024@b4ed526#diff-1bb2ab9ac4bfc2228b67aa44a05372fad1349775561b8e6915ad124b462664ff

Hope branch is created fine, yet if something should be changed - please let me know.

@lordofbikes
Copy link
Member

Thanks for the changes! I think this is a good base now to inspect code and work on your forks branch until we have it ready to merge.
With nearly 8000 lines added, it may take some time to get behind the code.
Don't worry about code style. Our huge mature code base has so many styles meanwhile that it's not worth the efforts to unify them.

Concerning the unused files I assume they're fragments from earlier features or test code and were forgotten to delete. We have to check them in history, if they were ever used at all. Probably they can be deleted, git remembers them anyway.
Maybe some are only used with specific preprocessor macros defined, that has to be checked too.
But this is additional work for another branch or the main repo itself, we should not mix up separate concerns.

The direct link to the branch: https://github.com/sand1024/LibreCAD/commits/layer_tree_view/

@Didier-H
Copy link

Hi there. This is exactly what I was looking for. How can I get a version with this widget to try it ?

@dongxuli-concord-design

Try to build from source.

I feel we should provide a simple script to build from scratch using any GitHub branch.

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

No branches or pull requests

5 participants