Maybe this is because --auto-init was introduced in version 1.3.3 but you are using 1.3.1. (changelog).

But if this really is the issue at least the error message should be better and list the unknown option instead of the URL.

Thank you, my bad

@EchoZulu No problem. This points to a problem: the wrong error message. So I’ll reopen this issue and retitle it, so we can fix this in a feature release.

I did some research on this. The wrong error message is caused by the processing of undefined options.

Everything that not matched previously is passed to this lines. And the first unknown thing is treated as the URL. The next unknown option produces the error since it is passed to print_error_and_die.

So in case of git-ftp push --hello-world -v "sftp://example.com" the URL is set to --hello-world and since the real URL is not known as an option and there already is a URL defined, the URL produces the error.

In my opinion, this could be fixed by setting the URL only if the unknown option is the very last option of the command:

# Line 1730
if [ -z "$URL" -a $# == 1 ]; then

According to the manual the URL must be the last part of the command so this would still meet the specification, wouldn’t it? (git-ftp <action> [<options>] [<url>])

Okay. It’s not that easy because the tests don’t match this specification 😆

Many of the tests execute a command structured like this:
git-ftp [<options>] [<url>] <action>

This is because the options and the url are stored in $GIT_FTP in the test script and the action is appended for each case.

So we could rewrite all the tests or find another solution to this issue.

Yes, I would like to keep that flexibility to mix up options. Many commands support that. These are the same:

cp -r a b
cp a b -r

Maybe we should check the option for a valid URL structure. So when it comes to --auto-init and doesn't know it, it would recognise that it's not a URL and complain straight away:

fatal: Unrecognised option or URL: --auto-init

What do you think?

Update: We can probably leave the message as is then. Just declaring the URL as one option:

fatal: Unrecognised option: --auto-init

Did I get you right?

Instead of setting the url from the first unknown option (current state) or assuming the url is always the last part of the command (my suggestion from above) we could do something like this:

# Line 1730
if [ -z "$URL" -a is_URL "$1" ]; then

Whatever the function is_URL then looks like.

I skipped this in my thoughts because I don’t know if it’s that easy to validate a URL in our use case. We have some protocols to support. What if the URL contains the path? Is it different for SFTP (: separator)? Do we have to do such a sophisticated validation or is a basic one enough?

But I agree. It would be the best and cleanest solution for this. Setting the URL only if it is one seams reasonable for me. 😉 And maybe a Regex check is enough to catch all cases.

But I think it would be good to adopt the error message, since then we can have a unrecognised option or an invalid URL. It would be quite confusing to get something like:

fatal: Unrecognised option: ft?://example.com/path

It should say

fatal: Unrecognised option or invalid URL: ft?://example.com/path

I think a regular expression would be good. It's not easy though. Technically, a URI requires the protocol but we don't enforce that and have a default protocol of ftp:. The other mandatory part is a path which could start with -. If our regex would allow a simple path like that then we would still recognise --auto-init as URL. I guess we can assume that a host needs to be given. And a host can't start with -. A username can start with that but then we need an @. So we can probably find a regex that is preventing this particular error and is not too restrictive.

Alternatively, we could try to run the URL parsing that we already have in Git-ftp and then see if we get any reasonable values out. So we would need at least a valid host after parsing the URL. Re-writing the current URL parsing is a good idea anyway. I often think that it's quite buggy but I wouldn't be able to give you examples now. There are probably some open issue for that.

Yeah, and that’s the reason why I tried to find a easier solution in the first place 😆 But the simplest solution is not always the best.

Running the URL parsing in this early state is a interesting idea, but could lead to other problems. What if the URL is given by command but other parts (user, path, …) are stored in a config? What if the option --scope is set in the command but not parsed yet?

So I’d try to find a regex in the first place.

Okay, I'm happy to go with a regex.

Turns out its actually not easy aka. super complicated to find a regex that matches everything we need. There are a lot of patterns out there, but none of the ones I tried covered all the cases we needed.

So I would propose to go with an easier approach: Define case where something is meant as an option (not a url) and exit in this cases. This is quite easy:

• 2 characters long AND first character is -
• longer but first two characters are --
  In this two simple cases I’d say “this is an option“ and treat everything else as a URL.

It’s not 100% perfect. As you said before a URL hostnames or domains can’t start with a - so we would be fine with this. If there is a protocol in the URL obviously everything is good ass well. But what about the pattern username:[password]@[host]? If the username starts with - or -- we could run into an issue theoretically, but think in the real world this scenario would be really rare, wouldn’t it?