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

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

Open
malberts opened this issue Aug 1, 2024 · 11 comments
Open

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

malberts opened this issue Aug 1, 2024 · 11 comments
Labels

Comments

@malberts
Copy link
Contributor

malberts commented Aug 1, 2024

Setup

  • MW version: 1.39.8
  • DB (MySQL etc.): 10.5.23-MariaDB-0+deb11u1
  • PHP version: 7.4.33 (apache2handler)
  • SMW version: 4.2.0
  • SRF version: 4.2.1
  • Browsers and versions (if applicable): n/a

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 malberts added the bug label Aug 1, 2024
@malberts
Copy link
Contributor Author

malberts commented Aug 2, 2024

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

@malberts
Copy link
Contributor Author

malberts commented Aug 2, 2024

2 years ago it seems like it was expected for SRF+Diagrams to return <br />:
4166b60

But that Diagrams-specific assertion was removed shortly thereafter:
71822ed#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
Copy link
Member

kghbln commented Aug 3, 2024

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

malberts commented Aug 5, 2024

@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
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

@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
Copy link
Contributor

@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.

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

No branches or pull requests

4 participants