ljos / sparql-mode

A SPARQL mode for emacs
GNU General Public License v3.0
62 stars 23 forks source link

Added support for variables in the query #32

Closed alf closed 9 years ago

alf commented 9 years ago

Very handy for creating named queries and then calling them with different arguments.

For example:

#+NAME: count-statements-in-graph
#+BEGIN_SRC sparql :var graph=""
  select count(*) where { graph <$graph> { ?s ?p ?o }}
#+END_SRC

#+CALL: count-statements-in-graph("http://example.com/graph")
ljos commented 9 years ago

Thank you for this! I think this will be an excellent addition to ob-sparql.

Though, I do have some minor issues that I would like you to correct. Firstly, the ?var and $var are the exact same variable in SPARQL. I think this could lead to confusion from some users if this isn't done correctly(, and by some users, I mean me. I am not very good at this type of context switching).

Another small issue is that you are breaking SPARQL syntax in your example, you have <> around the variable. I think we should instead have that as part of the input and not inside the query. The query then becomes

#+NAME: count-statements-in-graph
#+BEGIN_SRC sparql :var graph=""
  select count(*) where { graph $graph { ?s ?p ?o }}
#+END_SRC

#+CALL: count-statements-in-graph("<http://example.com/graph>")

What do you think?

Also, if you could edit the example or add another example in the README to show how the feature works, that would be nice.

I also don't like the forced to table when receiving CSVs. I think it should be configurable; it could even be the default, but I think :results should decide what the output should look like. (Maybe it already is able to do this by configuring :results?). Just for git-repo cleanliness it would be good if you would open a separate PR with any changes if you find the time to do this as well. Otherwise you can remove the commit and I will do it later when I have the time.

Thank you again for working on this. I really appreciate it. Hopefully I haven't scared you away by being too pedantic. Please tell me if you get into any problems working with the code.

alf commented 9 years ago

I did not intend to include other than the first commit in this pull request.

I completely agree that the convert to table change is way too intrusive. I did that as a quick and dirty solution for my own need and was planning on cleaning it up and use the :results option etc before I sent a pull request on that.

I'll close this pull request and open a new with just the templating stuff (including your requested changes). That'll have to wait for tomorrow though :)

alf commented 9 years ago

I've moved this out to separate branches and will send new pull requests soon.

Do you have any suggestions for what to use instead of $ for template substitution to avoid conflicts with sparql syntax?

I'm considering @ or %.

alf commented 9 years ago

Since @ is used for prefix and language tags while % is only used inside strings I'm going with % unless you have any better ideas.