remote_ls changes from @stewid's review

- Rename to remote_ls
- Check result of git2r_repository_open
- Use err for error codes
- Explicitly assign to err so it can be used for printing error messages
- Allocate, assign and set names in one statement
- Move statements after all variable definitions
- Add optional credentials parameter
- Fix documentation typo

Author: jimhester

NAMESPACE

@@ -9,8 +9,8 @@ export(is_commit)
 export(libgit2_features)
 export(libgit2_sha)
 export(libgit2_version)
-export(ls_remote)
 export(punch_card)
+export(remote_ls)
 exportClasses(cred_env)
 exportClasses(cred_ssh_key)
 exportClasses(cred_token)
@@ -81,7 +81,6 @@ exportMethods(is_merge)
 exportMethods(is_shallow)
 exportMethods(length)
 exportMethods(lookup)
-exportMethods(ls_remote)
 exportMethods(merge)
 exportMethods(merge_base)
 exportMethods(note_create)
@@ -98,6 +97,7 @@ exportMethods(push)
 exportMethods(references)
 exportMethods(reflog)
 exportMethods(remote_add)
+exportMethods(remote_ls)
 exportMethods(remote_remove)
 exportMethods(remote_rename)
 exportMethods(remote_set_url)

NEWS

@@ -3,7 +3,8 @@ git2r 0.11.0.9000
 
 NEW FEATURES
 
-* Add 'ls_remote' method to list references in a remote repository.
+* Add 'remote_ls' method to list references in a remote repository akin to the
+  `git ls-remote` command.
 
 * Add 'remote_set_url' method to set the remote's url in the
   configuration.

R/remote.r

@@ -354,40 +354,34 @@ setMethod("remote_url",
 
 ##' List references in a remote repository
 ##'
-##' Displays references available in a remote repository along with the associated commit IDs.
-##' @rdname ls_remote-methods
+##' Displays references available in a remote repository along with the
+##' associated commit IDs.  Akin to the 'git ls-remote' command.
+##' @rdname remote_ls-methods
 ##' @docType methods
-##' @param name Character vector with the with "remote" repository URL to query.
-##' @return Character vector for each reference with the associated commit IDs.
-##' @param repo an optional repository object used to store data temporarily.
+##' @param name Character vector with the "remote" repository URL to query or
+##the name of the remote if a \code{repo} argument is given.
+##' @param repo an optional repository object used if remotes are specified by name.
+##' @param credentials The credentials for the remote repository.
 ##' @keywords methods
+##' @return Character vector for each reference with the associated commit IDs.
 ##' @examples
 ##' \dontrun{
-##' ls_remote("https://github.com/ropensci/git2r")
+##' remote_ls("https://github.com/ropensci/git2r")
 ##' }
 ##' @export
-setGeneric("ls_remote",
-           signature = c("name", "repo"),
-           function(name, repo)
-           standardGeneric("ls_remote"))
+setGeneric("remote_ls",
+           signature = c("name"),
+           function(name,
+                    repo = git2r::init(tempdir()),
+                    credentials = NULL)
+           standardGeneric("remote_ls"))
 
-##' @rdname ls_remote-methods
-##' @export
-setMethod("ls_remote",
-          signature(name = "character",
-                    repo = "missing"),
-          function(name)
-          {
-              .Call(git2r_ls_remote, git2r::init(tempdir()), name)
-          }
-)
-##' @rdname ls_remote-methods
+##' @rdname remote_ls-methods
 ##' @export
-setMethod("ls_remote",
-          signature(name = "character",
-                    repo = "git_repository"),
-          function(name, repo)
+setMethod("remote_ls",
+          signature(name = "character"),
+          function(name, repo, credentials)
           {
-              .Call(git2r_ls_remote, repo, name)
+              .Call(git2r_remote_ls, name, repo, credentials)
           }
 )

man/ls_remote-methods.Rd

@@ -119,7 +119,7 @@ static const R_CallMethodDef callMethods[] =
     {"git2r_remote_rename", (DL_FUNC)&git2r_remote_rename, 3},
     {"git2r_remote_set_url", (DL_FUNC)&git2r_remote_set_url, 3},
     {"git2r_remote_url", (DL_FUNC)&git2r_remote_url, 2},
-    {"git2r_ls_remote", (DL_FUNC)&git2r_ls_remote, 2},
+    {"git2r_remote_ls", (DL_FUNC)&git2r_remote_ls, 3},
     {"git2r_repository_can_open", (DL_FUNC)&git2r_repository_can_open, 1},
     {"git2r_repository_discover", (DL_FUNC)&git2r_repository_discover, 1},
     {"git2r_repository_fetch_heads", (DL_FUNC)&git2r_repository_fetch_heads, 1},

src/git2r.c

@@ -392,69 +392,76 @@ SEXP git2r_remote_url(SEXP repo, SEXP remote)
     return url;
 }
 
-SEXP git2r_ls_remote(SEXP repo, SEXP name)
+/**
+ * Get the remote's url
+ *
+ * Based on https://github.com/libgit2/libgit2/blob/babdc376c7/examples/network/ls-remote.c
+ * @param repo S4 class git_repository
+ * @param name Character vector with URL of remote.
+ * @return Character vector for each reference with the associated commit IDs.
+ */
+SEXP git2r_remote_ls(SEXP name, SEXP repo, SEXP credentials)
 {
-    if (git2r_arg_check_string(name))
-        git2r_error(__func__, NULL, "'name'", git2r_err_string_arg);
-
     const char *name_ = CHAR(STRING_ELT(name, 0));
     SEXP result = R_NilValue;
     SEXP names = R_NilValue;
     git_remote *remote = NULL;
-    int error;
+    int err;
     const git_remote_head **refs;
     size_t refs_len, i;
     git_remote_callbacks callbacks = GIT_REMOTE_CALLBACKS_INIT;
-    git_repository *repo_ = NULL;
+    git2r_transfer_data payload = GIT2R_TRANSFER_DATA_INIT;
+    git_repository *repository = NULL;
 
-    repo_ = git2r_repository_open(repo);
+    if (git2r_arg_check_string(name))
+        git2r_error(__func__, NULL, "'name'", git2r_err_string_arg);
 
-    error = git_remote_lookup(&remote, repo_, name_);
-    if (error < 0) {
-      error = git_remote_create_anonymous(&remote, repo_, name_);
-      if (error < 0) {
-        goto cleanup;
-      }
-    }
+    if (git2r_arg_check_credentials(credentials))
+        git2r_error(__func__, NULL, "'credentials'", git2r_err_credentials_arg);
 
-    /**
-     * Connect to the remote and call the printing function for
-     * each of the remote references.
-     */
-    callbacks.credentials = git2r_cred_acquire_cb;
+    repository = git2r_repository_open(repo);
 
-    error = git_remote_connect(remote, GIT_DIRECTION_FETCH, &callbacks);
-    if (error < 0) {
-      goto cleanup;
-    }
+    if (!repository)
+        git2r_error(__func__, NULL, git2r_err_invalid_repository, NULL);
 
-    /**
-     * Get the list of references on the remote and print out
-     * their name next to what they point to.
-     */
-    if (git_remote_ls(&refs, &refs_len, remote) < 0) {
-      goto cleanup;
+    err = git_remote_lookup(&remote, repository, name_);
+    if (err < 0) {
+        err = git_remote_create_anonymous(&remote, repository, name_);
+        if (err < 0) {
+            goto cleanup;
+        }
     }
 
+    payload.credentials = credentials;
+    callbacks.payload = &payload;
+    callbacks.credentials = &git2r_cred_acquire_cb;
+
+    err = git_remote_connect(remote, GIT_DIRECTION_FETCH, &callbacks);
+    if (err < 0)
+        goto cleanup;
+
+    err = git_remote_ls(&refs, &refs_len, remote);
+    if (err < 0)
+        goto cleanup;
+
     PROTECT(result = allocVector(STRSXP, refs_len));
-    PROTECT(names = allocVector(STRSXP, refs_len));
+    setAttrib(result, R_NamesSymbol, names = allocVector(STRSXP, refs_len));
 
     for (i = 0; i < refs_len; i++) {
-      char oid[GIT_OID_HEXSZ + 1] = {0};
-      git_oid_fmt(oid, &refs[i]->oid);
-      SET_STRING_ELT(result, i, mkChar(oid));
-      SET_STRING_ELT(names, i, mkChar(refs[i]->name));
+        char oid[GIT_OID_HEXSZ + 1] = {0};
+        git_oid_fmt(oid, &refs[i]->oid);
+        SET_STRING_ELT(result, i, mkChar(oid));
+        SET_STRING_ELT(names, i, mkChar(refs[i]->name));
     }
-    setAttrib(result, R_NamesSymbol, names);
 
 cleanup:
-    if (repo_)
-        git_repository_free(repo_);
+    if (repository)
+        git_repository_free(repository);
 
     if (result != R_NilValue)
-      UNPROTECT(2);
+        UNPROTECT(1);
 
-    if (error)
+    if (err)
         git2r_error(__func__, giterr_last(), NULL, NULL);
 
     return(result);

src/git2r_remote.c

@@ -33,6 +33,6 @@ SEXP git2r_remote_remove(SEXP repo, SEXP remote);
 SEXP git2r_remote_rename(SEXP repo, SEXP oldname, SEXP newname);
 SEXP git2r_remote_set_url(SEXP repo, SEXP name, SEXP url);
 SEXP git2r_remote_url(SEXP repo, SEXP remote);
-SEXP git2r_ls_remote(SEXP repo, SEXP name);
+SEXP git2r_remote_ls(SEXP name, SEXP repo, SEXP credentials);
 
 #endif

src/git2r_remote.h

@@ -56,13 +56,13 @@ remote_remove(repo, "foobar")
 
 stopifnot(identical(remotes(repo), character(0)))
 
-refs <- ls_remote("https://github.com/ropensci/git2r")
+refs <- remote_ls("https://github.com/ropensci/git2r")
 stopifnot(length(refs) > 0)
 stopifnot(names(refs) > 0)
 stopifnot(any(names(refs) == "HEAD"))
 
 # an invalid URL should throw an error
-tools::assertError(ls_remote("bad"))
+tools::assertError(remote_ls("bad"))
 
 ## Cleanup
 unlink(path, recursive=TRUE)