Ted Zlatanov <t...@lifelogs.com> writes:
> None of these are a big deal, and Michal said he's working on libifying
> this anyhow:
>
> - making 'fill' a special operation is weird
Well, 'fill' is the only operation that mutates the credential structure
(i.e. the only one for which "git credential" emits an output to be
parsed), so you don't have much choice.
> - anchor the key regex to beginning of line (not strictly necessary)
Right. The greedyness of * ensures correction, but I like explicit
anchors ^...$ too.
> - sort the output tokens (after 'url' is extracted) so the output is
> consistent and testable
Why not, if you want to use the output of credential_write in tests. But
credential_write is essentially used to talk to "git credential", so the
important information is the content of the hash before credential_write
and after credential_read. They are unordered, but consistent and
testable.
>>> Maybe this can be merged with the netrc credential helper's
>>> read_credential_data_from_stdin() and print_credential_data()?
>
> MM> I don't know about the netrc credential helper, but I guess that's
> MM> another layer. The git-remote-mediawiki code is the code to call the
> MM> credential C API, that in turn may (or may not) call a credential
> MM> helper.
>
> Yup. But what you call "read" and "write" are, to the credential
> helper, "write" and "read" but it's the same protocol :) So maybe the
> names should be changed to reflect that, e.g. "query" and "response."
I don't think that would be a better naming. Maybe "serialize" and
"parse" would be better, but "query" would sound like it establishes the
connection and possibly reads the response to me.
> MM> One thing to be careful about: git-remote-mediawiki is currently a
> MM> standalone script, so it can be installed with a plain "cp
> MM> git-remote-mediawiki $somewhere/". One consequence of libification
> MM> is that it adds a dependency on the library (e.g. Git.pm). We should
> MM> be carefull to keep it easy for the user to install it (e.g. some
> MM> kind of "make install", or update the doc).
>
> I don't know--it's up to the `git-remote-mediawiki' maintainers...
That is, me ;-).
> But I think anywhere you have Git, you also have Git.pm, right?
Yes, but you have to find out where it is installed. Git's Makefile
hardcodes the path to Git.pm at build time, inserting one line in the
perl script:
use lib (split(/:/, $ENV{GITPERLLIB} || "$INSTLIBDIR"));
The same needs to be done for git-remote-mediawiki. As much as possible,
I'd rather avoid copy-pasting from Git's Makefile, so this means
extracting the perl part of Git's Makefile and make it available in
contrib/.
I'll try a patch in this direction.
> Maybe? But then you also have to look at whether Git.pm has the
> functionality you need...
If git-remote-mediawiki is installed from Git's source, I think it's OK
to assume that Git.pm will be up to date, but that would be even better
if we can issue a clean error message when the functions to be called do
not exist.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
