rhdunn / xquery-intellij-plugin

XQuery, XPath and XSLT Language Support for the IntelliJ IDE
https://rhdunn.github.io/xquery-intellij-plugin/
Apache License 2.0
25 stars 9 forks source link

Built-in Functions: BaseX #54

Closed rhdunn closed 5 years ago

rhdunn commented 7 years ago

Namespaces (http://docs.basex.org/wiki/Module_Library)

CanOfBees commented 6 years ago

@rhdunn hello (and thanks again for the pointers). This looks like an issue I can help with, but I have a question: how do you prefer to address changes in BaseX modules across BaseX versions; e.g. changes in the admin module from v8.0 to v8.3 to v9.0[1].

Are you wanting these variations approached similarly to how the MarkLogic XDMP dbg.xqy is written? Use the %a:since annotation and check for versions?

Cheers!

[1] BaseX Admin Module changelog

Edit: I created a couple of feature branches that might be acceptable [2, 3]. When you have some time to take a look, I'll be happy to open a PR and see what needs to change. [2]Admin Module functions, and [3]Archive Module functions

rhdunn commented 6 years ago

Yes, that's the general idea. There are other annotations as well (see https://github.com/rhdunn/xquery-intellij-plugin/blob/master/src/main/resources/builtin/reecedunn.co.uk/xquery/annotations.xqy). For example, https://github.com/rhdunn/xquery-intellij-plugin/blob/master/src/main/resources/builtin/marklogic.com/cts.xqy. The idea is to support these in the plugin, but that is not implemented yet.

The module files should be in a path matching the namespace. e.g. /builtin/basex.org/modules/archive.xqy. This allows the plugin to locate the functions from a namespace declaration only.

If the function has a different type signature across versions, I have been using things like cts:box-north($box as cts:box) (: as [5.0]xs:float [9.0]xs:numeric :). Similarly for generalised union values (e.g. $options (: as [5.0]element() [8.0](element()|map:map)? :)). I'm not sure how to properly deal with these.

CanOfBees commented 6 years ago

A follow-up question: would you prefer a "bulk" PR; e.g. all modules as a single PR, or would you prefer each module as separate PR?

rhdunn commented 6 years ago

Whichever works best for you. For the MarkLogic modules, I added them all at once for version 5.0, then updated by version (6.0, 7.0, ...). Adding them a module at a time is fine. I prefer self-contained commits, but for things like this I don't mind how they are organised.

CanOfBees commented 6 years ago

I'm planning to make the remainder of the modules a single PR. I'm sorry for the wave of small PRs - I hope you don't mind. The next one will take a bit but should get the rest in one.

Cheers!

CanOfBees commented 6 years ago

@rhdunn - another question, apologies. It looks like some of the argument names have changed, at least according to the wiki; e.g. older version of csv:parse vs newer version of same -- i.e.:

(: older version(s) :)
csv:parse($input as xs:string)
(: new version :)
csv:parse($string as xs:string?)

.Does it make any sense to indicate this in the declaration? I've indicated the type change[1], but I'm not sure if the argument name change matters. I'm inclined to think that it doesn't but wanted to check.

[1] CSV Module

Edit: additionally, some of the versions in the comments, e.g. [8.6.7] are best guesses, based on time of change and the version that was released at the time. If that's overkill, or too much "guess", please let me know and I'll pare those down.

Thank you!

rhdunn commented 6 years ago

I would personally use the newer argument name. However, either are fine.

Re: versions. I would go by the changelog in the documentation unless that is not accurate. If you are not sure on an exact version, a major/minor version should be fine.

CanOfBees commented 6 years ago

Thanks for the feedback.

Do you envision articulating all versions of a implementation, or is there a reasonable cutoff for older versions? I.e. go back to BaseX v1.0, or BaseX v7.8?

rhdunn commented 5 years ago

Ideally, the versions should match when the function was added/modified/removed to provide accurate information in the IDE.

Realistically, recording only the version information from the changelog should be sufficient (only going through the version history to look up the signature of deleted functions mentioned in the changelog).

NOTE: This includes going back as far as v1.0 where relevant.

rhdunn commented 5 years ago

Hi, I have some feedback for your ongoing work on this issue:

  1. The array module is already supported as it is a W3C function module -- see builtin/www.w3.org/2005/xpath-functions/array.xqy. You don't need to add a version for BaseX. NOTE: The same applies to other W3C function namespaces.
  2. It would be helpful to not commit invalid XQuery (e.g. in 7fcce309108d8223ea2f14b81cb84e5a34152c01) -- you can use git commit --amend to update an existing commit if you are saving incomplete work between edits.
  3. Some of the modules (e.g. crypto.xqy) have the IntelliJ template comment.
  4. Don't set the baseline version for since annotations if the function is not introduced then (e.g. for db.xqy).
  5. The file location should match the namespace URI (e.g. crypto.xqy should be in builtin/expath.org/ns).

NOTE: The other BaseX modules should be in builtin/basex.org/modules (note the .org in basex.org!)

CanOfBees commented 5 years ago

Hi - thanks so much for the feedback. I'll be a little more deliberate about how I'm approaching things from here - hopefully that will help avoid some of the issues you mention below.

Hi, I have some feedback for your ongoing work on this issue:

1. The `array` module is already supported as it is a W3C function module -- see `builtin/www.w3.org/2005/xpath-functions/array.xqy`. You don't need to add a version for BaseX. **NOTE:** The same applies to other W3C function namespaces.

2. It would be helpful to not commit invalid XQuery (e.g. in [7fcce30](https://github.com/rhdunn/xquery-intellij-plugin/commit/7fcce309108d8223ea2f14b81cb84e5a34152c01)) -- you can use `git commit --amend` to update an existing commit if you are saving incomplete work between edits.

Thanks for mentioning this. I haven't really explored that particular corner of git, but will definitely do so in the future.

3. Some of the modules (e.g. crypto.xqy) have the IntelliJ template comment.

Jeez. Mistakes like this make me think that getting rid of my computer completely would be the best course of action. Thanks for catching this.

4. Don't set the baseline version for since annotations if the function is not introduced then (e.g. for db.xqy).

5. The file location should match the namespace URI (e.g. crypto.xqy should be in `builtin/expath.org/ns`).

NOTE: The other BaseX modules should be in builtin/basex.org/modules (note the .org in basex.org!)

Where's the damned 'embarrassed' emoji?!

Again, I really appreciate your time (and help) with this. It's one thing to undertake the effort by yourself, but spot-checking my work and providing feedback is an incredibly valuable thing for me.

Cheers.

CanOfBees commented 5 years ago

Hi, I wonder if you could expand on the following:

Ideally, the versions should match when the function was added/modified/removed to provide accurate information in the IDE.

Realistically, recording only the version information from the changelog should be sufficient (only going through the version history to look up the signature of deleted functions mentioned in the changelog).

NOTE: This includes going back as far as v1.0 where relevant.

I think I have a better understanding of what you're wanting to accomplish here, but I have some concerns, specifically in regards to the BaseX database module. Based on historical changelog info from the wiki, the earliest entry for the db module is Dec. 2010, but there isn't any specific ChangeLog info until Feb. 2012 which only articulates changes for versions 7.0 and 7.1.

4. Don't set the baseline version for since annotations if the function is not introduced then (e.g. for db.xqy).

I'm positive I've got some bad data in here so I'm not trying to excuse my sloppiness, but this general problem was making me think treating v7.8 as a baseline would be acceptable; i.e. there would be some equivalency with the Extensions introduced in v7.8. However, as I mentioned, I'm seeing this differently now.

Unfortunately, I'm not sure how to handle/approach the lack of accurate change info prior to Feb. 2012/v7.(0|1). Would setting v7.0 as a baseline for the since annotation meet your approval?

Thanks!

rhdunn commented 5 years ago

This is what I meant by ideally vs realistically. For example, the MarkLogic version information only goes back to version 5.0.

As such, I am happy for 7.0 to be the oldest version in the since annotations.