-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Enable dd blocks by default and remove global setting #57036
Conversation
@@ -149,7 +149,7 @@ QgsVectorLayerAndAttributeModel::QgsVectorLayerAndAttributeModel( QgsLayerTree * | |||
const QgsVectorLayer *vLayer = qobject_cast< const QgsVectorLayer *>( QgsProject::instance()->mapLayer( id ) ); | |||
if ( vLayer ) | |||
{ | |||
mCreateDDBlockInfo[vLayer] = QgsDxfExportDialog::settingsDxfEnableDDBlocks->value(); | |||
mCreateDDBlockInfo[vLayer] = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be really handy to have a constant that can be used across all DXF Export classes to access this default value.
Just saying because I'm preparing another PR (see my local PR in the meantime), where I actually need the default value to improve code readability and maintenance (namely here to have something like if ( allowDataDefinedBlocks != DEFAULT_DXF_DATA_DEFINED_BLOCKS)
).
Without such constant, it would be too easy for someone to break something in the future regarding this default value.
I guess it could live in the dxfexport.h
or in the qgis.cpp.
Would that make sense for you @mhugent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sounds good to have it in dxfexport.h
d1675d8
to
5a23cc4
Compare
@mhugent, you had already implemented the |
I did. But you are right, I cannot find it in the PR anymore. Don't know if it got lost with one of the rebases |
@gacarrillor : I'll open a new PR for it. Do you want to first merge your other PR? |
If you mean #57016, yes, I'm currently rebasing (solving conflicts) with the changes you introduced in this PR and some from #57103, and hopefully it will soon be ready to merge. Once it's merged, I'll bring this one (which is currently against my own master) to the official QGIS repo. For that one it would be great to have the default constant implemented. To make things easier, I could ping you once it's been created against QGIS master and I'd appreciate if you could commit to it bringing the default constant. Sounds good to you? |
+1, waiting for your ping. I found out why it got lost. Implemented it on laptop A, then did the rebase from laptop B using force-push and without pulling first... |
Ouch!
Thank you @mhugent, I'll let you know! |
Enable dd blocks by default and remove global setting 'enable-datadefined-blocks'. This PR replaces #56984