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

Bug fix on create file on render_drawio. #1

Closed
wants to merge 1 commit into from

Conversation

Maggi-Andrea
Copy link
Contributor

Now we hash the 'filename' node.attributes instead of the keys of the
node.attributes dict.

Now we hash the 'filename' node.attributes instead of the keys of the
node.attributes dict.
@modelmat
Copy link
Owner

modelmat commented Jan 28, 2020

What was the bug?
I'll take a look at this is more detail tomorrow night.

@Maggi-Andrea
Copy link
Contributor Author

I have multipe draw but the extention resolve all of them with the same image, this because when you make the hash with hash_key = "".join(node.attributes).encode() you are performin the hash of the keys in node.attributes and not with their value (node.attributes is a dict). I changed the code to use the 'filename' value from the node.attributes becouse obviously each filename will be different for different directive.

@modelmat
Copy link
Owner

It might be better to do it off the attributes.values()

One major reason is that currently we have the format attribute which could change between filenames.
If I end up adding width and height this will also change the output file.

@Maggi-Andrea
Copy link
Contributor Author

Maggi-Andrea commented Jan 29, 2020

I see what do you mean.
I have tried that before to use attributes.values(), but I was getting an exception becouse not all value are jointable into the string, beacuse i.e. some values were list. Obviously this is fixable with a little bit of work.

I selected at first just the filename because this was enouth for me at the moment. Maybe it would be better, insted of just getting all the elements with the values(), get the element we are interested in - ['filename', 'width', ...] - that are responsible for changing the format of the rendered image.

Whatever I think you get the point where the bug is, it up to you selecting the approch you prefer.

@modelmat modelmat mentioned this pull request Feb 5, 2020
@modelmat
Copy link
Owner

modelmat commented Feb 5, 2020

I found a helpful method which will serve our purposes
https://github.com/docutils-mirror/docutils/blob/e88c5fb08d5cdfa8b4ac1020dd6f7177778d5990/docutils/nodes.py#L629-L632

I'm going to close this PR and add it to master.

@modelmat modelmat closed this Feb 5, 2020
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.

None yet

2 participants