👍 for the idea. I'm a little concerned that a function on *Configuration is printing to stderr though. I wonder if there's a better way to handle that. Ideally the commands themselves have full control over what gets sent to stdout or stderr. The lfs package is more like a library.

Yeah, I don't like writing directly to stderr from within what is effectively library code but there is some precedence (e.g. pointer_smudge.go). Another thought is to use the logging code in commands.go.

I think it makes sense for library code to return errors, and the commands package to log or output them correctly. I want to review and include this with a new v0.5.2 release next week. #281

Code-style counterproposal:

func (c *Configuration) Endpoint() Endpoint {
    e := c.endpoint()

    if u, err := url.Parse(e.Url); err == nil && u.User != nil {
        fmt.Fprintln(os.Stderr, "warning: configured LFS endpoint contains credentials")
    }

    return e
}

func (c *Configuration) endpoint() Endpoint {
    if url, ok := c.GitConfig("lfs.url"); ok {
        return Endpoint{Url: url}
    }

    if len(c.CurrentRemote) > 0 && c.CurrentRemote != defaultRemote {
        if e := c.RemoteEndpoint(c.CurrentRemote); len(e.Url) > 0 {
            return e
        }
    }

    return c.RemoteEndpoint(defaultRemote)
}

I think it makes sense for library code to return errors, and the commands package to log or output them correctly.

I looked into it and didn't see a best way to do that. The endpoint is never really exposed to the commands package and its usage seems to be deeply nested in package lfs.

I came to the same conclusion. I think an stderr message here is fine. If someone can come up with a better idea, I have no problem slowly refactoring things to support it.

I do want to change newEndpoint() into a function without a receiver though. I'll make a PR based on this one soon to get the idea across.

Incorporated suggested code style proposal.

Hey @ewbankkit, I noticed some issues with this when I actually reviewed the code. The Creds struct is used for git-credential specifically. In the case that a remote URL already has auth info, we don't need to even call git-credential. Since your patch returns a Creds populated from the url auth info, it will still call git credential approve to store the url auth username/pass in the configured git credential helper.

I fixed it in #407 #408, which builds on the new Endpoint constructors introduced in #403. It changes getCreds() to check apiUrl for url credentials. I was able to use your almost test verbatim.

Thanks for diving in and attempting this!