SemanticMediaWiki / SemanticResultFormats

Provides additional visualizations (result formats) for Semantic MediaWiki
https://www.semantic-mediawiki.org/wiki/Extension:Semantic_Result_Formats
Other
45 stars 75 forks source link

Graph format shows "<br />" when specifying nodelabel #846

Open malberts opened 1 month ago

malberts commented 1 month ago

Setup

Issue

Graphs with overridden nodelabel display <br /> instead of line breaks in the labels.

Steps to reproduce

Create a graph and specify |nodelabel=displaytitle.

Example on SMW sandbox: https://sandbox.semantic-mediawiki.net/wiki/Mise_en_place_d%27un_wiki_(small)

{{#ask:
 [[Process::Mise en place d'un wiki]]
 |?oui
 |?non
 |?has Successor=
 |?has Role=
 |format=graph
 |nodeshape=box
 |graphlink=yes
 |nodelabel=displaytitle
}}

Expected result: The labels in the graph should contain line breaks.

Observed result: The labels in the graph contain <br /> instead of line breaks.

Screenshot_20240801_202649

malberts commented 1 month ago

The unit test actually fails locally when using MW 1.39 and Diagrams: Screenshot_20240802_171211

malberts commented 1 month ago

2 years ago it seems like it was expected for SRF+Diagrams to return <br />: https://github.com/SemanticMediaWiki/SemanticResultFormats/pull/745/commits/4166b60ba2de7231d851d00e4113e617331b95d2

But that Diagrams-specific assertion was removed shortly thereafter: https://github.com/SemanticMediawiki/SemanticResultFormats/commit/71822ed6c963d6b2d5e83ed5359228236f78c863#diff-bf093da20db0230f90e042cbf438838858a9205b506e3209084ce7ac6e991cb7L72-L74

It is not actually clear to me what was the expected behavior. The only thing I know right now is the current behavior, as per this issue, does not seem to be correct.

kghbln commented 1 month ago

@alex-mashin Since you authored the change.

I'd expect to get some kind of line break whether with <br> or \n, or perhaps even better with white-space: pre-line; or white-space: pre-wrap; per https://stackoverflow.com/a/45178556

alex-mashin commented 1 month ago

Can't bring myself to writing unit tests (which, for this extension are next to meaningless anyway) for a big patch I made this spring, which is likely to remove this issue.

alex-mashin commented 1 month ago

You might want to have a look at my fork, though I last checked it under MW 1.35; and did some cosmetic changes afterwards, which I was unable to test so far.

malberts commented 1 month ago

@alex-mashin I did not have a look yet, but is your plan to merge that fork back into SRF? Or are you going to keep it separate?

alex-mashin commented 1 month ago

@alex-mashin I did not have a look yet, but is your plan to merge that fork back into SRF? Or are you going to keep it separate?

I planned to test it with MW 1.39, write some formal tests and submit a pull request.

gesinn-it-gea commented 1 month ago

If you use the current code base from master you can simply run "make ci" to run the tests. Change the Makefile or create an ".env" with the versions (matrix) you want to test. The same setup is run in CI.

alex-mashin commented 1 month ago

If you use the current code base from master you can simply run "make ci" to run the tests. Change the Makefile or create an ".env" with the versions (matrix) you want to test. The same setup is run in CI.

Thank you, but I need to write new tests for the graphviz format, before running them.

gesinn-it-gea commented 1 month ago

@alex-mashin tests are always welcome! Did you see https://github.com/SemanticMediaWiki/SemanticResultFormats/tree/master/tests/phpunit/Unit/Graph

I did not have time to look into the existing tests if they are complete, meaningful, working :-) ...

alex-mashin commented 1 month ago

@alex-mashin tests are always welcome! Did you see https://github.com/SemanticMediaWiki/SemanticResultFormats/tree/master/tests/phpunit/Unit/Graph

I did not have time to look into the existing tests if they are complete, meaningful, working :-) ...

1), 2): no; 3) yes.