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

GraphViz: add charset option to base64 image string. #18

Closed
wants to merge 1 commit into from
Closed

GraphViz: add charset option to base64 image string. #18

wants to merge 1 commit into from

Conversation

Ithilias
Copy link

No description provided.

@clue
Copy link
Member

clue commented Jan 18, 2017

Thanks for filing this PR!

However, the motivation is a bit unclear to me, so perhaps you can elaborate? When is this needed? When would one change the charset attribute?

@Ithilias
Copy link
Author

Browser will decode the base64 string with the default browser charset, which often is iso-8859-1.
It may not be the case that the svg file is encoded with that encoding. This change allows you to specify the encoding for the browser.

@clue
Copy link
Member

clue commented Jan 18, 2017

Thanks, I understand :-) Can you provide a reproducible test case for this?

@Ithilias
Copy link
Author

use Fhaculty\Graph\Graph;
use Graphp\GraphViz\GraphViz;

$graph = new Graph();
$graph->setAttribute('charset', 'UTF-8');

$modelvertex = $graph->createVertex("üuu");
$modelvertex->setAttribute('graphviz.color', 'blue');
$modeltypevertex = $graph->createVertex("äaa");
$edge = $modelvertex->createEdgeTo($modeltypevertex);
$edge->setAttribute('graphviz.color', 'grey');

$graphviz = new GraphViz();
$graphviz->setFormat("svg");
echo $graphviz->createImageHtml($graph);

This is an example which shows encoding problems with german umlauts.
encodingproblem

@clue
Copy link
Member

clue commented Jan 19, 2017

Thanks, I'll play around with this a bit 👍

At first sight, I'm tempted to promote (enforce) UTF-8 encoding here and remove the ctor argument, as I see little value in providing any other encoding.

I'll (try to) get back to this soon-ish.

@clue
Copy link
Member

clue commented Oct 9, 2018

@Ithilias Again thank you for filing this PR and for your patience!

I agree that this feature makes perfect sense and have just merged a slightly modified version with #27 (with proper attribution of course) :shipit: 🎉

I've added some documentation to emphasize that UTF-8 is the default. I've removed the constructor argument, one can now use the the charset attribute to change to a different encoding instead, but this is usually not recommended.

Again, thank you! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants