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

Allow free naming of project properties #60855

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jef-n
Copy link
Member

@jef-n jef-n commented Mar 4, 2025

Description

Allows:

<properties name="properties">
  <properties name="1">
    <properties name="2" type="QString">foo</properties>
  </properties>
</properties>

Data-driven properties names could violate that monstrous regular expression earlier (eg. using layer ids with leading digits as key was illegal) and trigger "Entry token invalid" for no too obvious reason (even when trying to remove a non-existing entry).

Turns

<properties>
  <foo>
    <bar type="QString">baz</bar>
  </foo>
</properties>

into

<properties name="properties">
  <properties name="foo">
    <properties name="bar" type="QString">baz</properties>
  </properties>
</properties>

@github-actions github-actions bot added this to the 3.44.0 milestone Mar 4, 2025
Copy link

github-actions bot commented Mar 4, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 8f84b3c)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 8f84b3c)

@jef-n jef-n force-pushed the arbitrary-property-names branch from 81b0ae9 to 90d927b Compare March 7, 2025 00:27
Allows:

<properties name="properties">
  <properties name="1">
    <properties name="2" type="QString">foo</properties>
  </properties>
</properties>

Data-driven properties names could violate that monstrous regular
expression earlier (eg. using layer ids with leading digits as key was
illegal) and trigger "Entry token invalid" for no too obvious reason
(even when trying to remove a non-existing entry).

Turns

<properties>
  <foo>
    <bar type="QString">baz</bar>
  </foo>
</properties>

into

<properties name="properties">
  <properties name="foo">
    <properties name="bar" type="QString">baz</properties>
  </properties>
</properties>
@jef-n jef-n force-pushed the arbitrary-property-names branch from 90d927b to 8f84b3c Compare March 7, 2025 00:32
@nirvn
Copy link
Contributor

nirvn commented Mar 7, 2025

@jef-n , we still need the monstrous regular expression in here (and the test that covers it). What's your plan to re-add that here?

@jef-n , sorry, I read this way too quickly, I was referring to this XML cleanup on project load: https://github.com/qgis/QGIS/pull/55041/files#diff-911caa2767015e8dc9678b56bebaa47c7852167695ec49e048ad2f384cb6e391R1847

Ignore me :)

@jef-n jef-n closed this Mar 7, 2025
@jef-n jef-n reopened this Mar 7, 2025
@jef-n jef-n closed this Mar 7, 2025
@jef-n jef-n reopened this Mar 7, 2025
@jef-n jef-n closed this Mar 8, 2025
@jef-n jef-n reopened this Mar 8, 2025
@jef-n jef-n closed this Mar 10, 2025
@jef-n jef-n reopened this Mar 10, 2025
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