fix: border case for deleteList

[cristianvasquez]

The following snippet:

  const dataset = rdf.dataset([
    rdf.quad(ns.ex.term, ns.ex.p, ns.ex.list)
  ])
  const cf = clownface({ term: ns.ex.term, dataset })
  cf.deleteList(ns.ex.p)

would make the current version of clownface hang.

The quick fix I came up with is to check if the list of elements to delete grows or not. (I'm not sure if that's enough)

[tpluscode]

Does this also intend to fix a looping case like

_:list
  rdf:first "foo" ;
  rdf:rest _:list ;
.

[cristianvasquez]

I'm not sure I understand the question, I just intended to fix that specific case.

[tpluscode]

Ok, maybe I was misled by the word "loop" in the test case name

[tpluscode]

I think this test needs a different title and an assertion for when it should succeed

What is the intended outcome? Should ex:p triple be removed?

[cristianvasquez]

I think you are right about the misleading title.

My first intention was to just avoid clownface hanging when the user tries to delete a non-list or non-well-formed list. (no ending :nil)

But now that you say it, we must define the desired outcome.

I see some branches of how I Imagine this particular operation could work:

• If it's a list
  • Clownface performs the operation (1)
• If it's not a list
  • Clownface throws an error (2)
  • Clownface ignores the operation (3)
• If it's a malformed list
  • Clownface throws an error (4)
  • Clownface ignores the operation (5)

Could be (1), (3) and (4) a reasonable contract?

[tpluscode]

I think I agree, with a small asterisk

"not a list" is technically a "malformed list" IMO, only such that the firs node if malformed.

<> 
  ex:notList [ ] ;
  ex:malformedList [
    rdf:first "element" ;
    rdf:rest [] ;
  ] .

I would propose a solution where a "malformed list" is ignored when it's already broken on the first node. If the malformation occurs on the rdf:rest predicate, then you'd throw for sure

[cristianvasquez]

So far, so good,

But on second thought, looking into the isList() and list() method at:

clownface/lib/Clownface.js

Line 140 in a31be80

list () {

I think these methods must be reused and their contracts replicated in the deletion operation. (for example, no multiple values for rdf:rest)

[cristianvasquez]

@tpluscode I added new border cases to the tests, and the method unfortunately got a bit complex. What do you think about this contract e3a43ff ?