Skip to content

Resolve Issue 75 #77

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.

Sign up for GitHub

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

Closed
james-d-mitchell wants to merge 2 commits into from
Closed

Resolve Issue 75 #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c1414b
Resolve Issue 75
james-d-mitchell Sep 29, 2026
29e83fe
Increase line length to 100
james-d-mitchell Sep 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gaplint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
disable:
- align-assignments
columns: 100
2 changes: 1 addition & 1 deletion PackageInfo.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Persons := [
Status := "dev",

SourceRepository := rec(Type := "git",
URL := "https://github.com/gap-packages/GraphvizForGAP"),
URL := "https://github.com/gap-packages/GraphvizForGAP"),
IssueTrackerURL := Concatenation(~.SourceRepository.URL, "/issues"),
PackageWWWHome := "https://gap-packages.github.io/GraphvizForGAP/",

Expand Down
3 changes: 2 additions & 1 deletion gap/dot.gd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ DeclareOperation("GraphvizSetName",
#! @Returns a &GAPGraphviz; node object
#! @Description Adds a node to the graph.
#! If a node with the same name is already present the operation fails.
#! What constitute a valid <A>node</A> name is found at https://graphviz.org/doc/info/lang.html
#! What constitute a valid <A>node</A> name is found at
#! https://graphviz.org/doc/info/lang.html
#! @BeginExampleSession
#! @EndExampleSession
DeclareOperation("GraphvizAddNode",
Expand Down
21 changes: 20 additions & 1 deletion gap/dot.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,29 @@ InstallMethod(GraphvizSetAttr,
"for a graphviz node or edge, object, and object",
[IsGraphvizNodeOrEdge, IsObject, IsObject],
function(x, name, value)
GraphvizAttrs(x)[String(name)] := String(value);
name := String(name);
value := String(value);
NormalizeWhitespace(name);
NormalizeWhitespace(value);
GraphvizAttrs(x)[name] := value;
return x;
end);

InstallMethod(GraphvizSetAttr,
"for a graphviz node or edge, and string",
[IsGraphvizNodeOrEdge, IsString],
function(x, str)
local attr;

attr := SplitString(str, "=");
if Length(attr) <> 2 then
ErrorFormatted("expected a string of the form \"attribute_name=value\", ",
"but the input string {} does not contain \"=\"", str);

@dvberkel dvberkel Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the input string contains more than 1 =, e.g. "i=like=bananas", it will also fail, with the message

expected a string of the form "attribute_name=value" but the input string i=like=bananas does not contain "="

This is obviously an incorrect statement.

An alternative could be

expected a string of the form "attribute_name=value" but the input string i=like=bananas does not adhere

@reiniscirpons reiniscirpons Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agreed with @dvberkel that its probably best to skip argument checking in the 2 argument form at the moment for simplicity. So we leave it up to the user to supply a reasonable 2nd argument and simply add it to the attrs array as is.

@james-d-mitchell james-d-mitchell Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that that is consistent with the other methods though, I'm off to teach until 2, so can't do anything right away. I think "parsing" the string is consistent with the other 2 arg SetAttr, which is a little further down the same file (and probably does a better job than the version in this PR).

fi;

return GraphvizSetAttr(x, attr[1], attr[2]);
end);

InstallMethod(GraphvizSetAttr,
"for a graphviz object with subobjects, object, and object",
[IsGraphvizGraphDigraphOrContext, IsObject, IsObject],
Expand Down
12 changes: 11 additions & 1 deletion tst/dot.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#############################################################################
##

#@local a, b, color, e, g, gv, label, n, shape, G, D
#@local a, b, color, e, g, gv, label, n, shape, G, D, innode, f
gap> START_TEST("graphviz package: dot.tst");
gap> LoadPackage("GraphvizForGAP", false);;

Expand Down Expand Up @@ -229,5 +229,15 @@ gap> GraphvizNodes(g); AsString(g);
rec( a := <graphviz node "a">, b := <graphviz node "b"> )
"//dot\ndigraph unexpected {\n\ta\n\tb\n\ta -> b\n\tb -> a\n}\n"

# Issue 75
gap> f := GraphvizDigraph("blah");
<graphviz digraph "blah" with 0 nodes and 0 edges>
gap> GraphvizSetAttr(f, "rankdir", "min");
<graphviz digraph "blah" with 0 nodes and 0 edges>
gap> innode := GraphvizAddNode(f, "in");
<graphviz node "in">
gap> GraphvizSetAttr(innode, "shape= \"none\"");
<graphviz node "in">

#
gap> STOP_TEST("graphviz package: dot.tst", 0);
20 changes: 10 additions & 10 deletions tst/examples/structs.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,16 @@ gap> GraphvizAddEdge(s, "struct1:f2", "struct3:here");
#
gap> AsString(s);
"//dot\ndigraph structs {\n\tnode [shape=\"plaintext\"] \n\tstruct1 [label=<<T\
ABLE BORDER=\"0\" CELLBORDER=\"1\" CELLSPACING=\"0\">\n<TR>\n<TD>left</TD>\n<T\
D PORT=\"f1\">middle</TD><TD PORT=\"f2\">right</TD>\n</TR>\n</TABLE>>]\n\tstru\
ct2 [label=<<TABLE BORDER=\"0\" CELLBORDER=\"1\" CELLSPACING=\"0\">\n<TR>\n<TD\
PORT=\"f0\">one</TD>\n<TD>two</TD>\n</TR>\n</TABLE>>]\n\tstruct3 [label=<<TAB\
LE BORDER=\"0\" CELLBORDER=\"1\" CELLSPACING=\"0\" CELLPADDING=\"4\">\n<TR>\n<\
TD ROWSPAN=\"3\">hello<BR/>world</TD>\n<TD COLSPAN=\"3\">b</TD>\n<TD ROWSPAN=\
\"3\">g</TD>\n<TD ROWSPAN=\"3\">h</TD>\n</TR>\n<TR>\n<TD>c</TD>\n<TD PORT=\"he\
re\">d</TD>\n<TD>e</TD>\n</TR>\n<TR>\n<TD COLSPAN=\"3\">f</TD>\n</TR>\n</TABLE\
>>]\n\tstruct1:f1\n\tstruct2:f0\n\tstruct1:f1 -> struct2:f0\n\tstruct1:f2\n\ts\
truct3:here\n\tstruct1:f2 -> struct3:here\n}\n"
ABLE BORDER=\"0\" CELLBORDER=\"1\" CELLSPACING=\"0\"> <TR> <TD>left</TD> <TD P\
ORT=\"f1\">middle</TD><TD PORT=\"f2\">right</TD> </TR> </TABLE>>]\n\tstruct2 [\
label=<<TABLE BORDER=\"0\" CELLBORDER=\"1\" CELLSPACING=\"0\"> <TR> <TD PORT=\
\"f0\">one</TD> <TD>two</TD> </TR> </TABLE>>]\n\tstruct3 [label=<<TABLE BORDER\
=\"0\" CELLBORDER=\"1\" CELLSPACING=\"0\" CELLPADDING=\"4\"> <TR> <TD ROWSPAN=\
\"3\">hello<BR/>world</TD> <TD COLSPAN=\"3\">b</TD> <TD ROWSPAN=\"3\">g</TD> <\
TD ROWSPAN=\"3\">h</TD> </TR> <TR> <TD>c</TD> <TD PORT=\"here\">d</TD> <TD>e</\
TD> </TR> <TR> <TD COLSPAN=\"3\">f</TD> </TR> </TABLE>>]\n\tstruct1:f1\n\tstru\
ct2:f0\n\tstruct1:f1 -> struct2:f0\n\tstruct1:f2\n\tstruct3:here\n\tstruct1:f2\
-> struct3:here\n}\n"

#
gap> STOP_TEST("graphviz package: examples/structs.tst");
Loading