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

Set dxfEnableDDBlocks in options dialog #56984

Closed
wants to merge 1 commit into from

Conversation

mhugent
Copy link
Contributor

@mhugent mhugent commented Mar 27, 2024

Allows to set the setting dxfEnableDDBlocks in the options dialog. The checkbox is in the datasource tab now, don't know if there is a better place.

Screenshot_20240328_124203

@mhugent mhugent requested a review from 3nids March 27, 2024 12:42
@github-actions github-actions bot added this to the 3.38.0 milestone Mar 27, 2024
Copy link

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit dfc50a5)

@gacarrillor gacarrillor added the DXF/DWG Relating to DXF or DWG importing/exporting label Mar 27, 2024
@DelazJ
Copy link
Contributor

DelazJ commented Mar 27, 2024

@mhugent can you please add a screenshot given that this involves a gui change? This will help the various contributors of docs and changelog figure out what it is about, without browsing the diff. Thanks.
Ps: for instance, I first thought "data source" was referring to the data source manager, not the tab in Options dialog.

@DelazJ DelazJ added Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels Mar 27, 2024
@qgis-bot
Copy link
Collaborator

@mhugent

This pull request has been tagged for the changelog.

  • The description will be harvested so please provide a "nearly-ready" text for the final changelog
  • If possible, add a nice illustration of the feature. Only the first one in the description will be harvested (GIF accepted as well)
  • If you can, it's better to give credits to your sponsor, see below for different formats.

You can edit the description.

Format available for credits
  • Funded by NAME
  • Funded by URL
  • Funded by NAME URL
  • Sponsored by NAME
  • Sponsored by URL
  • Sponsored by NAME URL

Thank you!

@qgis-bot
Copy link
Collaborator

@mhugent
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@mhugent
Copy link
Contributor Author

mhugent commented Mar 28, 2024

Here the screenshot. The option 'Enable data defined blocks in DXF export' is new

Screenshot_20240328_124203

@DelazJ
Copy link
Contributor

DelazJ commented Mar 28, 2024

Thanks @mhugent. May I suggest to instead add it to the first comment? That is the one that will be picked by the different actions.

@rouault
Copy link
Contributor

rouault commented Mar 31, 2024

The checkbox is in the datasource tab now, don't know if there is a better place.

I'm not a UI expert, but this doesn't feel right to me. All the options under the Data Source Handling group are related to generic open mechanisms. Here this is about specific DXF export. First: do we really need an option here? (my understanding is that it is already available in the DXF export dialog. why this one in particular and not the others?). and if we do, then perhaps have a dedicated "DXF export options" group ? Or possibly as a "sub-tab" (not sure about the terminology) similar to the "GDAL" one ?

@mhugent
Copy link
Contributor Author

mhugent commented Apr 2, 2024

@rouault : that makes sense, it should probably go somewhere else. It's just that I don't know where (a new dxf-Tab just for one setting seems a bit strange)

my understanding is that it is already available in the DXF export dialog. why this one in particular and not the others?

The other options only affect the current dxf export. The setting in the option dialog is more a user preference that data defined blocks should always be enabled by default in the dialog. Still it would be an option to have the checkbox at the bottom of the dxf dialog.

Another possibility would be to remove the data defined settings completely from the UI (also from the dxf dialog) and always build data defined blocks. It is even possible to write a block only if it is referenced at least by two features and insert the symbols which are only used once directly into the entities sections to minimize the file size.

Opinions?

@rouault
Copy link
Contributor

rouault commented Apr 2, 2024

The setting in the option dialog is more a user preference that data defined blocks should always be enabled by default in the dialog

and why not keeping the last user preference in the export dialog as the next default ? That way, this avoids the "duplication" of the option in different places. I believe this is done in a few places in the application. But I'm not sure if it is a best practice.

@mhugent
Copy link
Contributor Author

mhugent commented Apr 2, 2024

and why not keeping the last user preference in the export dialog as the next default

It can be set per layer in the export dialog. So should we check if it is set for all layers the last time and if yes, enable it for all layers the next time?

@gacarrillor
Copy link
Member

@mhugent, as far as I understand, you're attempting to set a global setting for an option that can be anyways modified on a case-by-case (per-layer) basis on the DXF Export dialog.

To avoid this global setting, perhaps it could be enough to think whether all Allow data defined symbol blocks checkboxes should be selected by default or not.

On the other hand, I intend to make this very per-layer setting remembered when closing and opening the dialog (fixing #56853). That is, if the same layer is present in both DXF Export sessions, its Allow data defined symbol blocks setting will be set as the last time.

Additionally, note that #56982 suggests that QGIS allows users to store all DXF Export settings to an XML file, so that the Allow data defined symbol blocks setting will also be remembered for particular layers even across machines and/or non-consecutive QGIS sessions.

I hope it helps.

@mhugent
Copy link
Contributor Author

mhugent commented Apr 2, 2024

To avoid this global setting, perhaps it could be enough to think whether all Allow data defined symbol blocks >checkboxes should be selected by default or not.

Ok, this is another solution. So if there are no objections, I'll enable the checkboxes by default and remove the global setting

@mhugent
Copy link
Contributor Author

mhugent commented Apr 4, 2024

So if there are no objections, I'll enable the checkboxes by default and remove the global setting

I've created a new PR, so closing this one: #57036

@mhugent mhugent closed this Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog Items that are queued to appear in the visual changelog - remove after harvesting DXF/DWG Relating to DXF or DWG importing/exporting Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants