Skip to content

Conversation

@Its-Just-Nans
Copy link
Contributor

Potential fix for #993

@LaurenzV
Copy link
Collaborator

Two things:

  1. I think this doesn't only apply to markers but all other types as well (for example clip paths, so we should probably not restrict the check for markers.
  2. I think it would be better to do the check earlier, like so:
    /// Returns an attribute value.
    pub fn attribute<T: FromValue<'a, 'input>>(&self, aid: AId) -> Option<T> {
        let value = self
            .attributes()
            .iter()
            .find(|a| a.name == aid)
            .map(|a| a.value.as_str())?;
        
        if value == "none" {
            return None;
        }
        
        match T::parse(*self, aid, value) {
            Some(v) => Some(v),
            None => {
                // TODO: show position in XML
                log::warn!("Failed to parse {} value: '{}'.", aid, value);
                None
            }
        }
    }

@Its-Just-Nans
Copy link
Contributor Author

Make sense, fixed

@LaurenzV LaurenzV changed the title remove warning for marker Don't emit warning when encountering link with none Dec 21, 2025
@Its-Just-Nans
Copy link
Contributor Author

The simple if value == "none" is not great looking from the test

Here is another fix

@LaurenzV
Copy link
Collaborator

There probably is some better fix for this, but I also can't think of anything off the top of my head that wouldn't require larger changes, so seems good to me. Thanks!

@LaurenzV LaurenzV merged commit 072148a into linebender:main Dec 21, 2025
5 checks passed
@sww1235
Copy link

sww1235 commented Dec 31, 2025

Thanks for fixing this so quickly!

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.

3 participants