-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[dxf] Allow users to override the layer name to be exported to DXF #57016
Conversation
@gacarrillor 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. Thank you! |
Imho, this can be expanded to direct the user to the QGIS Server tab as many users would never look there! You can also use |
Agreed. I opened this issue to get some feedback (I wasn't sure about the layer title provenance: metadata or QGIS Server). Looking at the code (and commit description), it's clear that the feature was really accounting for QGIS Server's properties.
|
I don't think we should ever use the settings from the server tab for non server purposes. We should use the values from the layer metadata instead. |
While I agree in principle, this is just making the status quo explicit if I understand correctly, so changing this now would not be backwards compatible (?) |
Couldn't we use QgsMapLayer::metadata().title() instead? |
Just switching will silently break projects which rely on the current behavior. What we could possibly do to move to a better world is: // TODO QGIS 4: no longer rely on server title
QString title = QgsMapLayer::metadata().title().isEmpty() ? serverTitle : QgsMapLayer::metadata().title(); And document the server title as legacy. |
76cad50
to
3b778d6
Compare
Rebasing... |
I'll do this in a follow up, and mark all the server related metadata properties in QgsMapLayer as deprecated after moving them to QgsMapLayerServerProperties 👍 |
@nyalldawson good move, but it doesn't affect the discussion here, does it? @gacarrillor looks like this needs another rebase |
No, it's not directly related. It just means I withdraw my earlier reservation 👍 |
…fLayerJob->layerTitle replaced by layerDerivedName, to avoid confusion with one of the final DXF layer name sources (layer metadata title, layer server properties title, layer name, layer's overridden name)
…attribute, 2: layer title, 3: overridden name, 4: layer name)
…nce precedence is until now not clear for them
3b778d6
to
a76439d
Compare
…t resolution with PR 57103)
Rebased, we are green! |
@gacarrillor In Processing, the name the user provides is kept along with the layer name. Helpful to know what they renamed. But in the export dialog, the layer name is no longer displayed if you renamed it. Would that not be confusing? No way to provide the same rendering? Then I am trying to guess based on the info above but I fail to get what has precedence between the naming options:
|
@DelazJ, valid observation. Fortunately, we have the tooltip, which shows both layer name and source. However, now that you mention it, I think it's worth exploring if we can add a reset button in the line edit, so that we can go back to the original layer name in one click.
The order is as follows:
Perhaps we should include it in the tooltip: "If no attribute is chosen, prefer layer title (set in layer properties) to layer overridden name and to layer name". |
Could be nice indeed.
Humm I'm not really convinced by this order. If I set the name override in the alg (or GUI) just before exporting from the same dialog, I would expect it to get used. Not a title configured in another dialog, maybe some time ago. Switching the order would allow to just override a few layer names and have the others use their title (*). I'm not sure such a combination is possible in the current scenario, a title would take precedence on the "local" overridden name. (*) does a layer necessarily have a title, either from metadata or server properties?
We should add tooltips to all these options and adapt the tooltip to their order. Also we could expand if necessary in the docs. |
…(by using the QgsFilterLineEdit with default value)
There you go. 👍
The current approach is flexible and is able to handle this case, i.e., if the option Note that the option
They are both optional. |
👏🏿
Actually, my scenario is the other way. I have layers with filled metadata (or server configured). And for this specific project export, I want to use a different name for one special layer but keep using the title for the others.
I find both options less optimal than just set the "override name" for the special layer, and check "use title" (which would apply to the not renamed layers). And all in all, what I think is that the option that is closest to where I hit "run" should have priority over the further. and in this case, "override name" is closer than layer title. |
…the former can be changed in the export dialog, whereas the latter must be done in the layer properties, therefore, overridden layer name is closer to the export intention and should take the precedence)
I think it makes a lot of sense, thanks for the suggestion. The order has been redefined as follows:
All GUI hints and unit tests have been updated accordingly. |
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.
Looks good to me, let's merge this @gacarrillor . Thanks for being responsive to suggestions and opinions raised in the PR, way to go :)
@gacarrillor |
Thanks to everyone who participated in this! 👍 |
This pull request has been tagged for the changelog.
You can edit the description. Format available for credits
Thank you! |
This PR makes it possible to override the output name of individual layers exported in the DXF Export dialog.
The corresponding processing algorithm's GUI and parameter were updated accordingly.
Tests were added for:
qgsprocessingparameterdxflayers
parameter and DXF layer's widget wrapper.Screencast App dialog
2024-03-19.18-11-48.mp4
Screencast Processing dialog
2024-03-25.10-54-11.mp4
UX hints for the users:
Funded by the QGIS user group Switzerland.