Re: [PATCH v8 3/7] git-remote-mediawiki: New git bin-wrapper for developement

2013-07-06 Thread Benoît Person
On 5 July 2013 09:04, Matthieu Moy  wrote:
> benoit.per...@ensimag.fr writes:
>
>> --- a/contrib/mw-to-git/Makefile
>> +++ b/contrib/mw-to-git/Makefile
>> @@ -2,6 +2,12 @@
>>  # Copyright (C) 2013
>>  # Matthieu Moy 
>>  #
>> +# To build and test:
>> +#
>> +#   make:
>> +# bin-wrapper/git mw preview Some_page.mw
>> +# bin-wrapper/git clone mediawiki::http://example.com/wiki/
>> +#
>
> The colon after "make" and the indentation look weird. Shouldn't this be
>
> +# To build and test:
> +#
> +#   make
> +#   bin-wrapper/git mw preview Some_page.mw
> +#   bin-wrapper/git clone mediawiki::http://example.com/wiki/
> +#
>
> ?

oops, yes definitely.
--
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


Re: [PATCH v7 2/7] wrap-for-bin: Make bin-wrappers chainable

2013-07-03 Thread Benoît Person
>> For now, GITPERLLIB is only used in that kind of statements:
>> use lib (split(/:/, $ENV{GITPERLLIB} || ... ));
>>
>> The trailing ':' does not really matter, split will ignore it.
>
> That may be true with the current use, but "For now" leaves funny
> taste, doesn't it?
definitely. For me, the issue was that we were trading that "funny
taste" for less
readable code in wrap-for-bin.sh but with that default-substitution
construct, it seems
worth it. (a huge if-block seemed even funnier in fact :) )

> It is not too much trouble to fix by writing
>
>   GITPERLLIB='@@BUILD_DIR@@/perl/blib/lib'"${GITPERLLIB:+:$GITPERLLIB}"
>
> and we will not have to be worried about future changes breaking
> today's assumption.
That construct is really nice :) did not know there was such a thing
in bash, thanks.
--
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


Re: [PATCH v7 2/7] wrap-for-bin: Make bin-wrappers chainable

2013-07-02 Thread Benoît Person
> Do we want to add that ':' unconditionally?  Could GITPERLLIB be
> empty?

For now, GITPERLLIB is only used in that kind of statements:
use lib (split(/:/, $ENV{GITPERLLIB} || ... ));

The trailing ':' does not really matter, split will ignore it.

With the current codebase, I think it's nicer to do it that way (makes
wrap-for-bin.sh more readable).
--
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


Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement

2013-06-29 Thread Benoît Person
> GIT_ROOT_DIR=../../..
> GIT_EXEC_PATH=$(cd "$(dirname "$0")" && cd ${GIT_ROOT_DIR} && pwd)
> GIT_MEDIAWIKI="$GIT_EXEC_PATH"/contrib/mw-to-git
> PATH="$GIT_MEDIAWIKI"/contrib/mw-to-git:"$PATH"
> GPLEXTRA="$GIT_MEDIAWIKI"/contrib/mw-to-git
>
> exec "${GIT_EXEC_PATH}/bin-wrappers/git" "$@"
Should I do that in a reroll ?

>>> Two possible alternatives:
>>>
>>>  - Is there a reason you would not want to "install" whatever Perl
>>>modules you want to "use" via GITPERLLIB mechanism to
>>>../../perl/blib/lib?
>> If we are making modifications to Git/Mediawiki.pm or even git-mw.perl
>> / git-remote-mediawiki.perl, installing them without proper testing
>> beforehand seems wrong.
>
> That's not "installing" (i.e. not "make install"), just a copy within
> the source tree. Same as what's done in Git's toplevel.
Oh right, sorry, read it too quickly :/ Well, I still find it weird to
copy untested files out of contrib/mw-to-git/ though.

Another idea crossed my mind: for now the test suite creates a symlink
of git-remote-mediawiki in the toplevel if it's not installed. It
would be better to use the bin-wrapper in the testsuite I think ?
(plus, with the current solution, it may have problems finding the
latest Git::Mediawiki.pm)
--
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


Re: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement

2013-06-27 Thread Benoît Person
> As far as I can tell, the only real reason why you need this and
> cannot use ../../bin-wrappers/git directly is because the GITPERLLIB
> it gives you only points at ../../perl/blib/lib and not this
> directory.
Plus (forgot to mention it in the other mail :/ ) it enables us to not
"copy" git-mw and git-remote-mediawiki at each "make" and stop us from
polluting the toplevel directory with those two scripts during
development.

> Two possible alternatives:
>
>  - Is there a reason you would not want to "install" whatever Perl
>modules you want to "use" via GITPERLLIB mechanism to
>../../perl/blib/lib?
If we are making modifications to Git/Mediawiki.pm or even git-mw.perl
/ git-remote-mediawiki.perl, installing them without proper testing
beforehand seems wrong.

>Perhaps it will interfere with the real
>installation step in ../../perl/Makefile?
I don't think so, but I am not an expert on what's going on in
../../perl/Makefile.

>If that is the case,
>then it is not a good idea, but otherwise, that would let you use
>../../bin-wrappers/git as-is.
>
>  - Perhaps we could do:
>
> GITPERLLIB="${GPLEXTRA+$GPLEXTRA:}@@BUILD_DIR@@/perl/blib/lib"
>
>in wrap-for-bin.sh, so that your instruction can become
>
> GPLEXTRA=$(pwd) ../../bin-wrappers/git whatever-mw-thing
>
>and you do not have to have this file?  We would also need to
>"unset GPLEXTRA" at the beginning of test-lib.sh if we were to do
>this.


> How does a developer (or user) use this script?  From your Makefile
> (e.g. "make test")?  Manually following some written instruction?
> In either case, the latter option feels a lot more sensible
> alternative without having to maintain this extra copy of wrap-for-bin
> that can easily go out of sync.
A developer uses it like any bin-wrapper : bin-wrapper/git mw preview
for instance. He can add it to its path while working on the scripts
etc ...

The best way to do it would be to factorise those new use cases
(adding new elements in $GITPERLLIB and $PATH) in the code that
generate bin-wrappers from wrap-for-bin.sh I think.

But it was weird to work on that in this serie which initial goal was
to add a preview tool for git-remote-mediawiki and ended up adding a
new perl package, a new 'git mw' command which handles subcommands,
...

In the end, this patch could be removed from the serie since it is
self-contained : it is not used by 3/5, 4/5, and 5/5.

Benoit
--
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


Re: contrib/mw-to-git/Git/Mediawiki.pm

2013-06-27 Thread Benoît Person
> Hmph.  Does it even need to be in-tree then?  Is it insufficient to
> run ../../git from that directory instead?
Well, the fact is we use Perl packages now (Git.pm and
Git::Mediawiki.pm in contrib/mw-to-git/Git/). The way we build perl
scripts in the toplevel's Makefile makes those packages accessible
only in $GITPERLLIB if it's defined and defaults to $INSTLIBDIR to
seek for installed version of those packages.

We use a bin-wrapper to define that $GITPERLLIB variable so that the
installed version gets bypassed by the one presents in the directory
defined in $GITPERLLIB.

> I just noticed that the script is not strictly a text file, ending
> with an incomplete line, by the way.
an incomplete line ?
--
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


Re: contrib/mw-to-git/Git/Mediawiki.pm

2013-06-27 Thread Benoît Person
Oops, so sorry :/

It's defintely doable since the lowercase 'git' is only a bin-wrapper
for git to ease development in contrib/mw-to-git/ .

Junio, Matthieu : should I resend a new version of my serie which
renames the 'git' (lowercase) file into something like 'git-dev' ?
(some comments directly mentionning the 'git' (lowercase) file needs
to be updated as well in the Makefile)

Benoit
--
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


Re: [PATCH v4 0/5] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-20 Thread Benoît Person
Ok, thanks for all the reviews,

Final version (I hope :) ) will come when all the mediawiki patches
will have graduated to 'master' then.

Benoit Person
--
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


Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread benoît person
> +sub split_email_list {
> +my(@list) = @_;
> +my @tmp;
> +my @emails;
> +   for (my $i = 0; $i <= $#list; $i++) {
> +   if ($list[$i] =~ /,/) {
> +   @emails = split(/,/, $list[$i]);
> +   } else {
> +   @emails = $list[$i];
> +   }
> +   # Removal of unwanted spaces
> +   for (my $j = 0; $j <= $#emails; $j++) {
> +   $emails[$j] =~ s/^\s+//;
> +   $emails[$j] =~ s/\s+$//;
> +   }
> +   @tmp = (@tmp, @emails);
> +   }
> +return(@tmp);
> +}
Why two regex ? You could do something like :
$emails[$j] =~ s/^\s+|\s+$//g;
to remove leading and trailing whitespaces at the same time.

I think it's better to use the builin 'push' function to concatenate
your two arrays:
push(@tmp, @emails);

Benoit Person
--
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


Re: [PATCH V3 1/4] git-mw: Introduction of GitMediawiki.pm

2013-06-18 Thread Benoît Person
On 17 June 2013 09:12, Matthieu Moy  wrote:
>>> Also, it seems to be only part of the solution. With your change, from
>>> contrib/mw-to-git/ and after running only "make",
>>>
>>> ./git-mw takes the installed version of GitMediawiki.pm in priority
>>>
>>> ../../bin-wrappers/git takes the installed version of git-mw only (i.e.
>>> does not know "git mw" if "make install" hasn't been ran).
>> Same thing as the documentation point, I think I am a bit lost in that
>> whole thing. I will re-look into it for the next version :/ .
>
> In short, the include path should contain both the *.pm file and the
> git- ones.
The fact is, for now, is there a way to test changes in
git-remote-mediawiki.perl without 'make install'-ing it ? I could not find one

So maybe in the "build-perl-script" of the toplevel Makefile we could add
something copying the script at the toplevel ?

And in GitMediawiki's Makefile, we let everything stay as is : copying *.pm
into /perl/blib/lib when building and copying it in installdir when installing ?

> I think you removed a newline from the end of the file. It's usually
> considered good practice to have this trailing newline (e.g. so that
> "cat file" in a terminal doesn't put your prompt after the last line).
> IIRC, it's actually required to call the file a "text file" according to
> POSIX.
That catch oO, thanks for the explanations.
>From my point of view, this could definitely be improved from:
> - perlcritic -2 *.perl
> + perlcritic -2 *.perl
> \ No newline at end of file
to something like that:
> - perlcritic -2 *.perl
> + perlcritic -2 *.perl
> \ removed newline at end of file
which gives more insights into why the line is considered "edited".

Benoit Person
--
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


Re: [PATCH V3 1/4] git-mw: Introduction of GitMediawiki.pm

2013-06-16 Thread Benoît Person
On 16 June 2013 22:18, Matthieu Moy  wrote:
> benoit.per...@ensimag.fr writes:
>
>> changes from the V2:
>>   - Add a way to test, without installation, code that uses GitMediawiki.pm.
>
> This still needs to be documented, even very quickly, somewhere in the
> code (e.g a comment in the Makefile).
Well, I think I will have to re-read some docs (and some earlier
reviews) about what to write in commit messages, in the emails, in the
code as comments and in the documentation ... I am just totally lost
right now :/ .

>> -build install clean:
>> +copy_pm:
>> + cp $(GIT_MEDIAWIKI_PM) $(GIT_ROOT_DIR)/perl/blib/lib/
>
> I already commented on this:
>
> http://thread.gmane.org/gmane.comp.version-control.git/227711/focus=227721
Oops, I forgot that one, so sorry :/ .

> Also, it seems to be only part of the solution. With your change, from
> contrib/mw-to-git/ and after running only "make",
>
> ./git-mw takes the installed version of GitMediawiki.pm in priority
>
> ../../bin-wrappers/git takes the installed version of git-mw only (i.e.
> does not know "git mw" if "make install" hasn't been ran).
Same thing as the documentation point, I think I am a bit lost in that
whole thing. I will re-look into it for the next version :/ .

>>  perlcritic:
>> - perlcritic -2 *.perl
>> + perlcritic -2 *.perl
>> \ No newline at end of file
>
> Please, avoid these whitespace-only changes. They create noise during
> review, and more potential conflicts.
For that one, I don't know why git assumes there is a change in it :
from what I see there is absolutely no whitespaces changes but maybe
my editor added some weird character somewhere ? I will look into that
for the next version ...

Thank you,

Benoit Person
--
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


Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script

2013-06-15 Thread Benoît Person
The V3 is ready, but I am still not sure about what is the best way to
do it for this issue though.

On 13 June 2013 15:01, Matthieu Moy  wrote:
> benoit.per...@ensimag.fr writes:
> How does the "make" Vs "make install" work? How does a developer run the
> tool without installing?
Well it does not work without installing but I think you know that now :)

> I first tried:
>
> $ ../../bin-wrappers/git mw
> git: 'mw' is not a git command. See 'git --help'.
>
> Then, this first seem OK:
>
> $ ./git-mw
> usage: git mw  
>
> git mw commands are:
> HelpDisplay help information about git mw
> Preview Parse and render local file into HTML
>
> BUT, this will take the installed GitMediawiki.pm if it is available,
> and we don't want this (if one hacks GitMediawiki.pm locally, one wants
> the new hacked to be taken into account without "make install"ing it).
>
> To understand better how it works, try adding this in git-mw.perl:
>
>   print "$_\n" for @INC;
>
> I get this:
>
> /home/moy/local/usr-squeeze/share/perl/5.14.2
> /home/moy/local/usr-squeeze/src/MediaWiki-API-0.39/blib/lib
> /etc/perl
> /usr/local/lib/perl/5.14.2
> /usr/local/share/perl/5.14.2
> /usr/lib/perl5
> /usr/share/perl5
> /usr/lib/perl/5.14
> /usr/share/perl/5.14
> /usr/local/lib/site_perl
> .
>
> The '.' is there, but it comes after the hardcoded
> /home/moy/local/usr-squeeze/share/perl/5.14.2 (which has to comes first,
> to let the install version be robust to whatever comes after).
Thanks for the explanations

> I think you need an equivalent of Git's toplevel bin-wrappers/git, or
> perhaps use the same bin-wrapper/git but let "make install" in
> contrib/mw-to-git/ install GitMediawiki.pm in perl/blib/lib
Typo s/make install/make/ ?

For now, I have implemented that one : each time you do `make`, if
there is changes in GitMediawiki.pm, it gets copied to $GITPERLLIB
(perl/blib/lib). But I am not sure it's the best approach here. If we
want something entirely self-contained for GitMediawiki, creating a
new git wrapper seems like the best way. But then, we could say that
since GitMediawiki Makefile uses Git toplevel Makefile, it's not
entirely self-contained :/ maybe it's the "copying file" that makes it
weird ?

> BTW, I just noticed we had a Git::SVN, so perhaps GitMediawiki should be
> Git::MediaWiki.
For that one, I am not really sure Git::Mediawiki makes more sense
than GitMediawiki. The point of the GitMediawiki.pm package is to
contain all the stuff for the bidirectionnal-thingy. So they are not
really Git-related, nor Mediawiki-related. Making it part of a "Git"
directory / namespace does not really feels right, even if it's how
it's done for SVN :/ .

Thank you for the all the reviews, (it works for Ensiwiki now \o/)

Benoit Person
--
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


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-11 Thread Benoît Person
The V2 is on the launchpad but I am still struggling with the code
factoring between git-mw.perl and git-remote-mediawiki.perl :/ .

On 9 June 2013 08:08, Jeff King  wrote:
>
> You could make a Git::MediaWiki.pm module, but installing that would
> significantly complicate the build procedure, and potentially be
> annoying for users. One trick I have done in the past is to concatenate
> bits of perl script together in the Makefile, like this:
>
>   foo: common.pl foo.pl
>   { \
> echo '$(PERL_PATH_SQ)' && \
> for i in $^; do \
>   echo "#line 1 $src" && \
> cat $src \
> done \
>   } >$@+
>   mv $@+ $@
>
> That would conflict a bit with the way we chain to git's Makefile,
> though. I suspect you could do something complicated like build "foo.pl"
> from "common.pl" and "foo-main.pl", then chain to git's Makefile to
> build "foo" from "foo.pl".

I've implemented this one for now but after a real-life meeting with
Matthieu Moy we discussed the possibility to build a GitMediawiki.pm
module. It seems more "clean" than the concatenation of perl scripts.
Plus, it would force people to limit side effects inside the functions
used in this package/utils file (I have in mind the mw_connect_maybe
function here and a couple of others which directly *hope* for global
vars to be set to a nice value before being called).

What I find bad in the concatenating-thingy is the mandatory rename of
git-mw.perl into something like git-mw.unmerged.perl and
git-remote-mediawiki.perl into git-remote-mediawiki.unmerged.perl.
Otherwise, like you said, it would be hard to chain to git's Makefile
after the merge.

For now, I have really no idea which one is the best. If I may ask,
what did you have in mind while saying:

> You could make a Git::MediaWiki.pm module, but installing that would
> significantly complicate the build procedure, and potentially be
> annoying for users.

?

Since my previous commit (ea07ec1 in next - use Git.pm functions for
credentials), git-remote-mediawiki.perl already depends on the proper
installation of the Git.pm package. In what ways the need for the
installation of yet another package (GitMediawiki.pm) would annoy a
user ?

Thank you for your time,

Benoit
--
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


Re: [PATCH v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style

2013-06-10 Thread Benoît Person
Well, I think next step would be to replace all those calls with
Git.pm `command`, `command_oneline` and `config``which take an array
of arguments after the "command". In the preview tool we use those but
I don't know if we will find the time to clean that up too in
git-remote-mediawiki :) .

Don't know though if it's better to mix that with this serie which is
mainly based on what perlcritic returns.

On 10 June 2013 18:41, Junio C Hamano  wrote:
> Matthieu Moy  writes:
>
>> Célestin Matte  writes:
>>
>>> @@ -1285,8 +1285,7 @@ sub get_mw_namespace_id {
>>>  # Look at configuration file, if the record for that namespace 
>>> is
>>>  # already cached. Namespaces are stored in form:
>>>  # "Name_of_namespace:Id_namespace", ex.: "File:6".
>>> -my @temp = split(/\n/, run_git("config --get-all remote."
>>> -. $remotename 
>>> .".namespaceCache"));
>>> +my @temp = split(/\n/, run_git("config --get-all 
>>> remote.${remotename}.namespaceCache"));
>>
>> I tend to prefer the former, as it avoids long lines (> 80 columns)
>
> But you split the name of a single variable across lines by doing so.
>
> You could split lines to make it a bit more readable.
>
> my @temp = split(/\n/,
> run_git("config --get-all 
> remote.${remotename}.namespaceCache"));
>
> It still overflows the 80-column limit, but the "namespaceCache" is
> the only thing that overflows; not much harm is done.
>
> This is totally outside of the topic of "coding-style" series, but I
> would be more concerned about the use of ${remotename}, though.  Has
> it (and in general the parameters to all calls to run_git()) been
> sanitized for shell metacharacters?  If we had a variant of run_git
> that took an array of command line arguments given to git, you could
> do this
>
> my @temp = split(/\n/,
> run_git([qw(config --get-all),
> "remote.${remotename}.namespaceCache")]);
>
> which would be safer and easier to read.
>
>>> @@ -1339,8 +1338,7 @@ sub get_mw_namespace_id {
>>>
>>>  # Store explicitely requested namespaces on disk
>>>  if (!exists $cached_mw_namespace_id{$name}) {
>>> -run_git("config --add remote.". $remotename
>>> -.".namespaceCache \"". $name .":". $store_id ."\"");
>>> +run_git(qq(config --add remote.${remotename}.namespaceCache 
>>> "${name}:${store_id}"));
>>
>> Same.
> --
> 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
--
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


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Benoît Person
On 9 June 2013 08:08, Jeff King  wrote:
> I also wonder if it would be useful to be able to specify not only files
> in the filesystem, but also arbitrary blobs. So in 4b above, you could
> "git mw preview origin:page.mw" to see the rendered version of what
> upstream has done.

Hum, so `git mw preview origin:page.mw` would just do the get
request to the remote mediawiki, save it locally and - maybe - load it
in the browser ? Is it really better than just opening the browser and
typing the right URL ?

Currently, this URL is one click away when you have preview file loaded
in a web browser.

>> It works but a couple of points trouble me:
>> 1-  I had to copy two functions from `git-remote-mediawiki.perl`, I don't
>> really know how we could "factorize" those things ? I don't think it 
>> makes
>> much sense to create a package just for them ?
>
> You could make a Git::MediaWiki.pm module, but installing that would
> significantly complicate the build procedure, and potentially be
> annoying for users. One trick I have done in the past is to concatenate
> bits of perl script together in the Makefile, like this:
>
>   foo: common.pl foo.pl
>   { \
> echo '$(PERL_PATH_SQ)' && \
> for i in $^; do \
>   echo "#line 1 $src" && \
> cat $src \
> done \
>   } >$@+
>   mv $@+ $@
>
> That would conflict a bit with the way we chain to git's Makefile,
> though. I suspect you could do something complicated like build "foo.pl"
> from "common.pl" and "foo-main.pl", then chain to git's Makefile to
> build "foo" from "foo.pl".

ok, thanks a lot.

>> 2-  The current behavior is to crash if the current branch do not have an
>> upstream branch on a valid mediawiki remote. To find that specific 
>> remote,
>> it runs `git rev-parse --symbolic-full-name @{upstream}` which will 
>> return
>> something like `/refs/remotes/$remote_name/master`.
>>   2a-  maybe there is a better way to find that remote name ?
>
> If you just care about the remote name and not the name of the local
> branch, you can just ask for
>
>   my $curr_branch = `git symbolic-ref HEAD`;
>   my $remote = `git config "branch.$curr_branch.remote"`;
>   my $url = `git config "remote.$remote.url"`;
>
> Of course you would want some error checks and probably some chomp()s in
> there, too.

The fact is, `git symbolic-ref HEAD` result would have to be parsed in order
to extract the current branch name like I currently extract the remote name.
So, is it really better than `git rev-parse --symbolic-full-name @{upstream}` ?

>>   2b-  would it be useful to add a fallback if that search fails ? searching
>>for a valid mediawiki remote url in all the remotes returned by
>>`git remote` for instance ?
>
> That is probably OK as long as there is only one such remote, and it
> would help the case where you have branched off of a local branch (so
> your upstream remote is ".").  If there are two mediawiki remotes,
> though, it would make sense to simply fail, as you don't know which to
> use. But I'd expect the common case by far to be that you simply have
> one such remote.

Well, I thought that `git mw preview` could provide an interactive mode
where, if the first search fails, it would find all the mediawiki remotes, and
offers to the user a way to choose the remote ?

Benoit Person

> --
> 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
--
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


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Benoît Person
Matthieu Moy  writes:
> Same question here. I'd expect "git mw preview" in a mediawiki workflow
> to do what "pdflatex foo && evince foo.pdf" do in a latex workflow: see
> in rendered form what I've been doing.
>
> In a latex flow, if I want to see how my local changes merge with the
> remote ones, I do "git merge && pdflatex", and I'd do the same with "git
> mw".

In fact, I should not have used "merge" to describe how the two contents
(page template + new parsed content) are combined together. For
now, the code simply replaces the template page's text content (the one
retrieved from the remote) with the new one. It does not really care if
the remote has changes or not. (And, to be honest, I did not thought
about that issue ;) ).

But, like both of you said : in a typical workflow, the merging would be
left to the user so the current behavior is fine I think ?

>> I also wonder if it would be useful to be able to specify not only files
>> in the filesystem, but also arbitrary blobs. So in 4b above, you could
>> "git mw preview origin:page.mw" to see the rendered version of what
>> upstream has done.
> Next step could even be "git mw diff $from $to", using the wiki to
> render the diff. Not a priority, but could be funny.

I searched in the Mediawiki API if there was a way to diff from a stored
revision and raw text content but I've found nothing :/ . We could make
a little "hack" to do that by saving as a new revision the local content,
and use the "DeleteRevision"-thingy from Mediawiki [1] to hide this
useless revision but it would floods the remote DB and usually users
to not have the permission to use that tool. So, for now I would say
it's a no-go :/ .

[1] http://www.mediawiki.org/wiki/Manual:RevisionDelete

Benoit Person

> --
> 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
--
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


Re: [PATCH v2 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma

2013-06-08 Thread Benoît Person
The major drawback of using perl `constant` is the fact that they do
not interpolate like variables in most of the contexts (those who
automatically quotes barewords). Like said on Perl Monks here [1], you
have to do some "tricks" depending on the context in which you're
using the constant and that's not really "clean".

But maybe trading clean for another package is not worth it. I just
searched for alternatives way of doing it and none seems to be in the
core packages so maybe we should just drop this.

[1] http://www.perlmonks.org/?node_id=11

On 8 June 2013 05:23, Jeff King  wrote:
> On Fri, Jun 07, 2013 at 11:42:03PM +0200, Célestin Matte wrote:
>
>> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl 
>> b/contrib/mw-to-git/git-remote-mediawiki.perl
>> index 4893442..e60793a 100755
>> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
>> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
>> @@ -26,21 +26,21 @@ use IPC::Open2;
>>  use Readonly;
>
> What does this series apply on top of? The existing version in "master"
> does not have "use Readonly" in it at all. The first version of your
> series introduced that line, but here it is shown as an existing line.
> Did you miss a commit when putting your patches together?
>
>>  # Mediawiki filenames can contain forward slashes. This variable decides by 
>> which pattern they should be replaced
>> -use constant SLASH_REPLACEMENT => "%2F";
>> +Readonly my $SLASH_REPLACEMENT => "%2F";
>
> What advantage does this have over "use constant"? I do not mind
> following guidelines from perlcritic if they are a matter of style, but
> in this case there is a cost: we now depend on the "Readonly" module,
> which is not part of the standard distribution. I.e., users now have to
> deal with installing an extra dependency. Is it worth it?
>
> -Peff
> --
> 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
--
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


contributing to git-mediawiki

2013-05-31 Thread Benoît Person
Hello,

We are another team from Ensimag (Célestin MATTE & Benoit PERSON) who
will contribute to Git and more specifically to Git-Mediawiki for our
one-month school project - and possibly more. We already have a couple
of basic patches in local and will submit them in the upcoming days.
After that, we will start working on more useful features. As a first
start, we thought about these two ideas posted in the Git-Mediawiki’s
issue tracker:
- A mechanism to preview local modifications in the browser (with
the wiki style) -- https://github.com/moy/Git-Mediawiki/issues/7
- Support for “push-by-rev” to increase push performance on some
wikis -- https://github.com/moy/Git-Mediawiki/issues/6

If you have any suggestions, ideas, guidances, feel free to share them with us.

Best regards,
Célestin MATTE & Benoit PERSON
--
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