git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Git Open Source Weekend London 11th/12th November
@ 2017-11-01 16:36 Thomas Gummerer
  2017-11-04  9:28 ` Jeff King
  2017-11-05 12:42 ` Git Open Source Weekend London 11th/12th November Patrick Steinhardt
  0 siblings, 2 replies; 12+ messages in thread
From: Thomas Gummerer @ 2017-11-01 16:36 UTC (permalink / raw)
  To: Git Mailing List; +Cc: hkleynhans, Jeff King

Hi,

Bloomberg is hosting an Open Source Weekend in London on the 11th
& 12th November 2017 to contribute to the Git project.  We have
also confirmed that Peff will be amongst the mentors on hand to
guide attendees through their efforts!

Some of you may notice that we tried to organize this earlier in the
year, but unfortunately had to postpone it.  For this time around we
are further along in planning, and it's definitely happening :)

For those unfamiliar with Bloomberg Open Source weekends: These
events provide a great way for those who love working on code to
give back to the community. Come and help make difference to a
great project!

There will be tasks provided by the mentors, or bring your own if
you already have a great idea.  Also if you can't attend the weekend
and can think of a project which you would like tackled at this
event please let me know.  Obviously the projects should be
completable inside a weekend.

Normally attendees work in small groups on a specific task to
prevent anyone from getting stuck. Per usual, Bloomberg will
provide the venue, mentors, snacks and drinks.  Bring your
enthusiasm (and your laptop!) and come share in the fun!  The
event is also open to everyone, so feel free to pass on the
invite!

The event is free of charge, but please ensure that you are able
to attend the event before registering.  That will greatly help
us with accurate numbers for catering so that we don't create
unwanted waste!

You can register for the event here:

https://go.bloomberg.com/attend/invite/git-sprint-hosted-bloomberg/

Whether you already are a contributor (as probably most people on
this list are) or interested in starting to contribute to git or
have some friends that you'd like to get to contribute to git, it
would be great to see you at the event.

If you have any further questions feel free to get in touch.

- Thomas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Git Open Source Weekend London 11th/12th November
  2017-11-01 16:36 Git Open Source Weekend London 11th/12th November Thomas Gummerer
@ 2017-11-04  9:28 ` Jeff King
  2017-11-04 17:15   ` Philip Oakley
  2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
  2017-11-05 12:42 ` Git Open Source Weekend London 11th/12th November Patrick Steinhardt
  1 sibling, 2 replies; 12+ messages in thread
From: Jeff King @ 2017-11-04  9:28 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Git Mailing List, hkleynhans

On Wed, Nov 01, 2017 at 04:36:24PM +0000, Thomas Gummerer wrote:

> Normally attendees work in small groups on a specific task to
> prevent anyone from getting stuck. Per usual, Bloomberg will
> provide the venue, mentors, snacks and drinks.  Bring your
> enthusiasm (and your laptop!) and come share in the fun!  The
> event is also open to everyone, so feel free to pass on the
> invite!

I think it will help if the experienced members of the community (both
those who will be at the event and not) can come up with some possible
topics to work on (though of course I'd be glad for participants to come
with their own itch to scratch).

We've started using the #leftoverbits tag to allow searching in the
archive:

  https://public-inbox.org/git/?q=leftoverbits

Some of those have since been completed, but others are left open.
There's not really a master list, but it's a potential source for
digging for gold (well, if you want to call leftover bugs gold :) ).

I started a list over the summer of larger items that people might want
to pick up. Here it is in no particular order:

 - the pager.<cmd> config is mis-designed, because our config keys
   cannot represent all possible command names (e.g., case folding and
   illegal characters). This should be pager.<cmd>.enable or similar.
   Some discussion in (this message and the surrounding thread):

     https://public-inbox.org/git/20170711101942.h2uwxtgzvgguzivu@sigill.intra.peff.net/

   But I think you could find more by searching the archive.

 - ditto for alias.* config, which has the same syntax problems.

 - auto-gc is sometimes over-anxious to run if you have a lot of
   unreachable loose objects. We should pack unreachables into a single
   pack. That solves the --auto problem, and is also way more efficient.
   The catch is expiration. Some discussion here (and especially
   down-thread):

     https://public-inbox.org/git/20170711101942.h2uwxtgzvgguzivu@sigill.intra.peff.net/

 - git-config's "--get-color" is unlike all the other types in that it
   takes a "default" value if the config key isn't set. This makes it annoyingly
   inconsistent, but there's also no way to ask Git to interpret other
   values (e.g., you might want it to expand "--path" or an "--int"). It
   would be nice to have a general "--default" option so you could do:

     # same as git config --get-color color.diff.old red
     git config --default red --color color.diff.old

   or

     # not currently possible to ask git to interpret "10k"
     git config --default 10k --int some.integer.key

 - git's internal config can parse expiration dates (see
   parse_expiry_date()), but you can't do so from git-config. It should
   probably have a type for "--expiry-date" (which would of course be
   more useful with the --default option above).

 - there's no efficient way to ask git-config for several keys with a
   specific type (or even multiple different types).  You can use
   "--list" to get their strings, but you can't get any interpretation
   (like colors, integers, etc). Invoking git-config many times can have
   a noticeable speed impact for a script. There should probably be a
   "--stdin" mode (or maybe "--get-stdin" if we would one day want to
   have a "--set-stdin") that takes a list of keys, optional types, and
   optional defaults (that "--default" again!) and outputs them to
   stdout.


Those were just sitting on my ideas list. I'm happy to go into more
detail if anybody's interested in discussing any of them. Some of them
may be half-baked.

And of course I'd be happy if people wanted to contribute more items to
the list.

-Peff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Git Open Source Weekend London 11th/12th November
  2017-11-04  9:28 ` Jeff King
@ 2017-11-04 17:15   ` Philip Oakley
  2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
  1 sibling, 0 replies; 12+ messages in thread
From: Philip Oakley @ 2017-11-04 17:15 UTC (permalink / raw)
  To: Jeff King, Thomas Gummerer; +Cc: Git Mailing List, hkleynhans

From: "Jeff King" <peff@peff.net>
> On Wed, Nov 01, 2017 at 04:36:24PM +0000, Thomas Gummerer wrote:
>
>> Normally attendees work in small groups on a specific task to
>> prevent anyone from getting stuck. Per usual, Bloomberg will
>> provide the venue, mentors, snacks and drinks.  Bring your
>> enthusiasm (and your laptop!) and come share in the fun!  The
>> event is also open to everyone, so feel free to pass on the
>> invite!
>
> I think it will help if the experienced members of the community (both
> those who will be at the event and not) can come up with some possible
> topics to work on (though of course I'd be glad for participants to come
> with their own itch to scratch).
>
> We've started using the #leftoverbits tag to allow searching in the
> archive:
>
>  https://public-inbox.org/git/?q=leftoverbits
>
> Some of those have since been completed, but others are left open.
> There's not really a master list, but it's a potential source for
> digging for gold (well, if you want to call leftover bugs gold :) ).
>
> I started a list over the summer of larger items that people might want
> to pick up. Here it is in no particular order:
>
> - the pager.<cmd> config is mis-designed, because our config keys
>   cannot represent all possible command names (e.g., case folding and
>   illegal characters). This should be pager.<cmd>.enable or similar.
>   Some discussion in (this message and the surrounding thread):
>
>
> https://public-inbox.org/git/20170711101942.h2uwxtgzvgguzivu@sigill.intra.peff.net/
>
>   But I think you could find more by searching the archive.
>
> - ditto for alias.* config, which has the same syntax problems.
>
> - auto-gc is sometimes over-anxious to run if you have a lot of
>   unreachable loose objects. We should pack unreachables into a single
>   pack. That solves the --auto problem, and is also way more efficient.
>   The catch is expiration. Some discussion here (and especially
>   down-thread):
>
>
> https://public-inbox.org/git/20170711101942.h2uwxtgzvgguzivu@sigill.intra.peff.net/
>
> - git-config's "--get-color" is unlike all the other types in that it
>   takes a "default" value if the config key isn't set. This makes it
> annoyingly
>   inconsistent, but there's also no way to ask Git to interpret other
>   values (e.g., you might want it to expand "--path" or an "--int"). It
>   would be nice to have a general "--default" option so you could do:
>
>     # same as git config --get-color color.diff.old red
>     git config --default red --color color.diff.old
>
>   or
>
>     # not currently possible to ask git to interpret "10k"
>     git config --default 10k --int some.integer.key
>
> - git's internal config can parse expiration dates (see
>   parse_expiry_date()), but you can't do so from git-config. It should
>   probably have a type for "--expiry-date" (which would of course be
>   more useful with the --default option above).
>
> - there's no efficient way to ask git-config for several keys with a
>   specific type (or even multiple different types).  You can use
>   "--list" to get their strings, but you can't get any interpretation
>   (like colors, integers, etc). Invoking git-config many times can have
>   a noticeable speed impact for a script. There should probably be a
>   "--stdin" mode (or maybe "--get-stdin" if we would one day want to
>   have a "--set-stdin") that takes a list of keys, optional types, and
>   optional defaults (that "--default" again!) and outputs them to
>   stdout.
>
>
> Those were just sitting on my ideas list. I'm happy to go into more
> detail if anybody's interested in discussing any of them. Some of them
> may be half-baked.
>
> And of course I'd be happy if people wanted to contribute more items to
> the list.
>

A few I've seen recently are:

* The provison of a `git resolve -X <ours|theirs> -- <pathspec>` command to
simplify the manual resolution of remaining conflicts.
https://public-inbox.org/git/8737615iu5.fsf@javad.com/  Sergey Organov: How
to re-merge paths differently?

* (Git for Windows/HFS): Detect directory capitalisation changes when
switching branches, and rename them correctly on case preserving, case
insensitive file systems. Optimisation: If the underlying tree is identical
then do not update the modified dates.
https://github.com/git-for-windows/git/issues/1333 Chuck Lu: Folder name
should be case sensitive when switch branches.

* (Git for Windows/HFS) (long standing):
detect branch name capitalisation issues
- may need a struct to carry both the filename and pathname down the
different parts of the code base so that the FS name of the requested
ref/heads/ can be checked and warned.
e.g. https://github.com/git-for-windows/git/issues/852 "`git checkout head`
inconsistent behavior"
- ('head' finds 'HEAD', but also 'branch' finds 'Branch');
https://github.com/git-for-windows/git/issues/852#issuecomment-239675187 ->
"rambling notes" for partial analysis.

https://github.com/git-for-windows/git/issues/752 "git checkout creating
tracking branch using case-insensitive match?"
- Is this part of the same problem?

* Documentation:

There's always the newby role at the hackathon of collating
all the "what does this command do/mean?" questions that could be resolved
by simple updates, or capturing locally written explanations to improve the
manual pages. (easy patch practice)

Perhaps see how `git rerere` could be better explained and integrated onto
the other man pages so that more folk naturally know of it and use it. (see
also the `git resolve` question above)

Also for case sensitivity documentation
https://github.com/git-for-windows/git/issues/908#issuecomment-325116189

--
Philip


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable}
  2017-11-04  9:28 ` Jeff King
  2017-11-04 17:15   ` Philip Oakley
@ 2017-11-05 11:58   ` Martin Ågren
  2017-11-05 11:58     ` [PATCH 1/4] t7006: document that `pager.foo` can be partially preserved Martin Ågren
                       ` (4 more replies)
  1 sibling, 5 replies; 12+ messages in thread
From: Martin Ågren @ 2017-11-05 11:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git, hkleynhans, Thomas Gummerer

I'm not posting this for inclusion (yet), but because I read this:

On 4 November 2017 at 10:28, Jeff King <peff@peff.net> wrote:
>  - the pager.<cmd> config is mis-designed, because our config keys
>    cannot represent all possible command names (e.g., case folding and
>    illegal characters). This should be pager.<cmd>.enable or similar.
>    Some discussion in (this message and the surrounding thread):
>
>      https://public-inbox.org/git/20170711101942.h2uwxtgzvgguzivu@sigill.intra.peff.net/
>
>    But I think you could find more by searching the archive.

I'm posting four patches I have on this to save others from redoing my
work and findings. These patches feel a bit incomplete, which is why I
put them to the side some time ago (and eventually forgot about them).

In particular, they do not teach `--paginate` to use the pager
configured by `pager.foo.command`. It is already now possible to use
`pager.foo` to say "I don't want you to page, but if I later give you
`pager.foo=true`, this is the pager I want you to use". That does not
work with `--paginate`, but this can all be explained -- indeed, we
document that `--paginate` overrules `pager.foo`.

If we teach `--paginate` to respect `pager.foo.command`, it seems that
we would either 1) introduce a small (and possibly hard to understand
and explain) difference between the old-style and the new-style
pager-configuration or 2) knowingly change the behavior of `--paginate`
with `pager.foo` or 3) knowingly change the behavior of
`pager.foo=false` as documented in the first patch.

I think there's great value to being able to say "this is the same as
this, and that is the same as that", but that might get muddied by "oh,
except if you use `--paginate`".

If someone wants to pick these up and bring them to completion, great!
If not and if I or someone else feels confident about which way to go,
then I can revisit these.

Martin

Martin Ågren (4):
  t7006: document that `pager.foo` can be partially preserved
  pager: refactor `pager_command_config()`
  pager: introduce `pager.*.command` and `.enable`
  pager: make `pager.foo.command` imply `.enable=true`

 Documentation/config.txt  | 19 +++++++++
 Documentation/git-tag.txt |  3 +-
 Documentation/git.txt     |  2 +-
 t/t7006-pager.sh          | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 pager.c                   | 16 +++++++-
 5 files changed, 134 insertions(+), 4 deletions(-)

-- 
2.15.0.415.gac1375d7e


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] t7006: document that `pager.foo` can be partially preserved
  2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
@ 2017-11-05 11:58     ` Martin Ågren
  2017-11-05 11:58     ` [PATCH 2/4] pager: refactor `pager_command_config()` Martin Ågren
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Martin Ågren @ 2017-11-05 11:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git, hkleynhans, Thomas Gummerer

Assuming a blank slate, consider the following configuration:

[pager]
	foo = some-command
	foo = false

Now `git -c pager.foo=true foo` will page using `some-command`. This
might have been intended, or is perhaps just a side-effect of the
implementation. In any case, it could be useful and someone might rely
on it, either knowingly (as above) or not (if these lines are spread out
across the configuration).

However, `git --paginate foo` will *not* use `some-command`. That
matches the documentation (Documentation/git.txt).

Upcoming commits will expand on how paging for `git foo` can be
configured. Those commits mustn't change how `pager.foo` behaves, so add
tests for these two cases.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 t/t7006-pager.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index f0f1abd1c..e890b2f64 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -117,6 +117,24 @@ test_expect_success TTY 'git config uses a pager if configured to' '
 	test -e paginated.out
 '
 
+test_expect_success TTY 'configuration remembers pager across boolean changes' '
+	echo paging >expected &&
+	test_unconfig pager.config &&
+	test_terminal git -c pager.config="echo paging" \
+			  -c pager.config=false \
+			  -c pager.config \
+			  config --list >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success TTY '--paginate does not respect inactivated pager' '
+	rm -f paginated.out &&
+	test_terminal git -c pager.config=bad \
+			  -c pager.config=false \
+			  --paginate config --list &&
+	test -e paginated.out
+'
+
 test_expect_success TTY 'configuration can enable pager (from subdir)' '
 	rm -f paginated.out &&
 	mkdir -p subdir &&
-- 
2.15.0.415.gac1375d7e


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] pager: refactor `pager_command_config()`
  2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
  2017-11-05 11:58     ` [PATCH 1/4] t7006: document that `pager.foo` can be partially preserved Martin Ågren
@ 2017-11-05 11:58     ` Martin Ågren
  2017-11-05 11:58     ` [PATCH 3/4] pager: introduce `pager.*.command` and `.enable` Martin Ågren
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Martin Ågren @ 2017-11-05 11:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git, hkleynhans, Thomas Gummerer

`pager_command_config()` checks for the config `pager.<cmd>`. In the
next commit, we will want to also look for some strings on the form
`pager.<cmd>.foo`.

Refactor the code to verify upfront that the string starts with
"pager.<cmd>" and then check that the remainder is the empty string.
This makes it easy to look for other remainders in the next patch.

While at it, before assigning to `value`, free any old value we might
already have picked up.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 pager.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/pager.c b/pager.c
index 92b23e6cd..8968f26f1 100644
--- a/pager.c
+++ b/pager.c
@@ -191,14 +191,19 @@ struct pager_command_config_data {
 static int pager_command_config(const char *var, const char *value, void *vdata)
 {
 	struct pager_command_config_data *data = vdata;
-	const char *cmd;
+	const char *cmd, *remainder;
+
+	if (!skip_prefix(var, "pager.", &cmd) ||
+	    !skip_prefix(cmd, data->cmd, &remainder))
+		return 0;
 
-	if (skip_prefix(var, "pager.", &cmd) && !strcmp(cmd, data->cmd)) {
+	if (!*remainder) {
 		int b = git_parse_maybe_bool(value);
 		if (b >= 0)
 			data->want = b;
 		else {
 			data->want = 1;
+			free(data->value);
 			data->value = xstrdup(value);
 		}
 	}
-- 
2.15.0.415.gac1375d7e


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] pager: introduce `pager.*.command` and `.enable`
  2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
  2017-11-05 11:58     ` [PATCH 1/4] t7006: document that `pager.foo` can be partially preserved Martin Ågren
  2017-11-05 11:58     ` [PATCH 2/4] pager: refactor `pager_command_config()` Martin Ågren
@ 2017-11-05 11:58     ` Martin Ågren
  2017-11-05 11:58     ` [PATCH 4/4] pager: make `pager.foo.command` imply `.enable=true` Martin Ågren
  2017-11-06 10:48     ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Jeff King
  4 siblings, 0 replies; 12+ messages in thread
From: Martin Ågren @ 2017-11-05 11:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git, hkleynhans, Thomas Gummerer

`pager.foo` conflates two concepts: how and whether `git foo` should
page. In particular, it can not be used to change *how* to page without
possibly also affecting *whether*.

Teach Git about two new config items, `pager.foo.command` and
`pager.foo.enable`.

Make this interact sanely with the existing `pager.foo`. For example,
make sure that `pager.foo=false` does not cause us to forget about a
command already configured through `pager.foo.command`, so that the
given pager command can be "re-activated" using
`pager.foo[.enable]=true`.

Where Documentation/ refers to `pager.tag`, write "the `pager.tag[.*]`
configuration options". In config.txt, `pager.blame` is mentioned more
as an example and it describes precisely the situation where one will
want to use the old mechanism, so leave that instance unchanged.

For symmetry with how `--paginate` disrespects any pager that might have
been configured with `pager.foo`, do the same for `pager.foo.command`.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/config.txt  | 17 +++++++++++
 Documentation/git-tag.txt |  3 +-
 Documentation/git.txt     |  2 +-
 t/t7006-pager.sh          | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 pager.c                   |  5 ++++
 5 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1ac0ae6ad..72558cc74 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2460,6 +2460,23 @@ pager.<cmd>::
 	or `--no-pager` is specified on the command line, it takes
 	precedence over this option.  To disable pagination for all
 	commands, set `core.pager` or `GIT_PAGER` to `cat`.
++
+This is a less flexible alternative to `pager.<cmd>.command` and
+`pager.<cmd>.enable`. Using it with a boolean does the same as using
+`pager.<cmd>.enable`. Using it with a command does the same as using
+`pager.<cmd>.command` and `pager.<cmd>.enable=true`.
+
+pager.<cmd>.command::
+	Specifies the pager to use for the subcommand.
+	Whether the pager should be used is configured using
+	`pager.<cmd>.enable` or `pager.<cmd>=<boolean>`.
+
+pager.<cmd>.enable::
+	A boolean which turns on or off pagination of the output of a
+	particular Git subcommand when writing to a tty. If `--paginate`
+	or `--no-pager` is specified on the command line, it takes
+	precedence over this option.  To disable pagination for all
+	commands, set `core.pager` or `GIT_PAGER` to `cat`.
 
 pretty.<name>::
 	Alias for a --pretty= format string, as specified in
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 956fc019f..9f9f33409 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -210,7 +210,8 @@ it in the repository configuration as follows:
     signingKey = <gpg-keyid>
 -------------------------------------
 
-`pager.tag` is only respected when listing tags, i.e., when `-l` is
+The `pager.tag[.*]` configuration options are only
+respected when listing tags, i.e., when `-l` is
 used or implied. The default is to use a pager.
 See linkgit:git-config[1].
 
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7a1d629ca..0a2eff7a6 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -99,7 +99,7 @@ foo.bar= ...`) sets `foo.bar` to the empty string which `git config
 -p::
 --paginate::
 	Pipe all output into 'less' (or if set, $PAGER) if standard
-	output is a terminal.  This overrides the `pager.<cmd>`
+	output is a terminal.  This overrides the `pager.<cmd>[.*]`
 	configuration options (see the "Configuration Mechanism" section
 	below).
 
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index e890b2f64..6966627dd 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -588,4 +588,77 @@ test_expect_success 'command with underscores does not complain' '
 	test_cmp expect actual
 '
 
+test_expect_success 'setup' '
+	sane_unset PAGER GIT_PAGER GIT_PAGER_IN_USE &&
+	test_unconfig core.pager &&
+
+	git rev-list HEAD >rev-list &&
+	sed "s/^/foo:/" rev-list >expect &&
+
+	PAGER="cat >paginated.out" &&
+	export PAGER &&
+
+	test_unconfig pager.log &&
+	test_unconfig pager.rev-list
+'
+
+test_expect_success TTY 'configuration with .enable works' '
+	rm -f paginated.out &&
+	test_terminal git -c pager.log.enable=false log &&
+	! test -e paginated.out
+'
+
+test_expect_success TTY '--paginate overrides .enable+.command' '
+	rm -f paginated.out &&
+	test_terminal git -c pager.log.command=bad -c pager.log.enable=false \
+			  --paginate log &&
+	test -e paginated.out
+'
+
+test_expect_success TTY '--no-pager overrides .enable' '
+	rm -f paginated.out &&
+	test_terminal git -c pager.rev-list.enable --no-pager rev-list HEAD &&
+	! test -e paginated.out
+'
+
+test_expect_success TTY '.enable discards non-boolean' '
+	test_must_fail git -c pager.log.enable=bad log
+'
+
+test_expect_success TTY 'configuration remembers .command as .enable flips' '
+	>actual &&
+	test_terminal git -c pager.rev-list.command="sed s/^/foo:/ >actual" \
+			  -c pager.rev-list.enable=false \
+			  -c pager.rev-list.enable \
+			  rev-list HEAD &&
+	test_cmp expect actual
+'
+
+test_expect_success TTY 'configuration remembers old-style command as .enable flips' '
+	>actual &&
+	test_terminal git -c pager.rev-list="sed s/^/foo:/ >actual" \
+			  -c pager.rev-list.enable=false \
+			  -c pager.rev-list.enable \
+			  rev-list HEAD &&
+	test_cmp expect actual
+'
+
+test_expect_success TTY 'old-style config can override .enable' '
+	>actual &&
+	test_terminal git -c pager.rev-list.command="sed s/^/foo:/ >actual" \
+			  -c pager.rev-list.enable=false \
+			  -c pager.rev-list \
+			  rev-list HEAD &&
+	test_cmp expect actual
+'
+
+test_expect_success TTY 'old style config can override .command+.enable' '
+	>actual &&
+	test_terminal git -c pager.rev-list.command=bad \
+			  -c pager.rev-list.enable=false \
+			  -c pager.rev-list="sed s/^/foo:/ >actual" \
+			  rev-list HEAD &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/pager.c b/pager.c
index 8968f26f1..c8a6a01d8 100644
--- a/pager.c
+++ b/pager.c
@@ -206,6 +206,11 @@ static int pager_command_config(const char *var, const char *value, void *vdata)
 			free(data->value);
 			data->value = xstrdup(value);
 		}
+	} else if (!strcmp(remainder, ".command")) {
+		free(data->value);
+		data->value = xstrdup(value);
+	} else if (!strcmp(remainder, ".enable")) {
+		data->want = git_config_bool(var, value);
 	}
 
 	return 0;
-- 
2.15.0.415.gac1375d7e


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] pager: make `pager.foo.command` imply `.enable=true`
  2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
                       ` (2 preceding siblings ...)
  2017-11-05 11:58     ` [PATCH 3/4] pager: introduce `pager.*.command` and `.enable` Martin Ågren
@ 2017-11-05 11:58     ` Martin Ågren
  2017-11-06 10:48     ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Jeff King
  4 siblings, 0 replies; 12+ messages in thread
From: Martin Ågren @ 2017-11-05 11:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git, hkleynhans, Thomas Gummerer

If `pager.foo.command` gets configured and there is no configuration
(yet) saying whether we should page, act as if `pager.foo.enable=true`.

This means that a lone `pager.foo` can always be written as a lone
`pager.foo.command` or `pager.foo.enable`.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/config.txt | 2 ++
 t/t7006-pager.sh         | 7 +++++++
 pager.c                  | 2 ++
 3 files changed, 11 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 72558cc74..91cc8b110 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2470,6 +2470,8 @@ pager.<cmd>.command::
 	Specifies the pager to use for the subcommand.
 	Whether the pager should be used is configured using
 	`pager.<cmd>.enable` or `pager.<cmd>=<boolean>`.
+	Implies `pager.<cmd>.enable=true` unless `pager.<cmd>.enable`
+	or `pager.<cmd>` have already been given.
 
 pager.<cmd>.enable::
 	A boolean which turns on or off pagination of the output of a
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 6966627dd..c5246a57d 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -625,6 +625,13 @@ test_expect_success TTY '.enable discards non-boolean' '
 	test_must_fail git -c pager.log.enable=bad log
 '
 
+test_expect_success TTY 'configuration with .command turns on paging' '
+	>actual &&
+	test_terminal git -c pager.rev-list.command="sed s/^/foo:/ >actual" \
+			  rev-list HEAD &&
+	test_cmp expect actual
+'
+
 test_expect_success TTY 'configuration remembers .command as .enable flips' '
 	>actual &&
 	test_terminal git -c pager.rev-list.command="sed s/^/foo:/ >actual" \
diff --git a/pager.c b/pager.c
index c8a6a01d8..0e037abf4 100644
--- a/pager.c
+++ b/pager.c
@@ -209,6 +209,8 @@ static int pager_command_config(const char *var, const char *value, void *vdata)
 	} else if (!strcmp(remainder, ".command")) {
 		free(data->value);
 		data->value = xstrdup(value);
+		if (data->want == -1)
+			data->want = 1;
 	} else if (!strcmp(remainder, ".enable")) {
 		data->want = git_config_bool(var, value);
 	}
-- 
2.15.0.415.gac1375d7e


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: Git Open Source Weekend London 11th/12th November
  2017-11-01 16:36 Git Open Source Weekend London 11th/12th November Thomas Gummerer
  2017-11-04  9:28 ` Jeff King
@ 2017-11-05 12:42 ` Patrick Steinhardt
  1 sibling, 0 replies; 12+ messages in thread
From: Patrick Steinhardt @ 2017-11-05 12:42 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Git Mailing List, hkleynhans, Jeff King

[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]

On Wed, Nov 01, 2017 at 04:36:24PM +0000, Thomas Gummerer wrote:
> Hi,
> 
> Bloomberg is hosting an Open Source Weekend in London on the 11th
> & 12th November 2017 to contribute to the Git project.  We have
> also confirmed that Peff will be amongst the mentors on hand to
> guide attendees through their efforts!
> 
> Some of you may notice that we tried to organize this earlier in the
> year, but unfortunately had to postpone it.  For this time around we
> are further along in planning, and it's definitely happening :)
> 
> For those unfamiliar with Bloomberg Open Source weekends: These
> events provide a great way for those who love working on code to
> give back to the community. Come and help make difference to a
> great project!
> 
> There will be tasks provided by the mentors, or bring your own if
> you already have a great idea.  Also if you can't attend the weekend
> and can think of a project which you would like tackled at this
> event please let me know.  Obviously the projects should be
> completable inside a weekend.
> 
> Normally attendees work in small groups on a specific task to
> prevent anyone from getting stuck. Per usual, Bloomberg will
> provide the venue, mentors, snacks and drinks.  Bring your
> enthusiasm (and your laptop!) and come share in the fun!  The
> event is also open to everyone, so feel free to pass on the
> invite!
> 
> The event is free of charge, but please ensure that you are able
> to attend the event before registering.  That will greatly help
> us with accurate numbers for catering so that we don't create
> unwanted waste!
> 
> You can register for the event here:
> 
> https://go.bloomberg.com/attend/invite/git-sprint-hosted-bloomberg/
> 
> Whether you already are a contributor (as probably most people on
> this list are) or interested in starting to contribute to git or
> have some friends that you'd like to get to contribute to git, it
> would be great to see you at the event.
> 
> If you have any further questions feel free to get in touch.
> 
> - Thomas

Hi,

nice to hear, thanks for organizing. Due to having moved to
London just that week I'd love to take part.

As I'm a core contributor of libgit2, I am interested in bringing
forward libgit2 at that occasion. Would that be welcome or should
participants keep strictly to the core git project? Just asking
as I'd like to take part but have a rather long list of topics
for libgit2 on my backlog which I'd like to tackle myself.

Regards
Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable}
  2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
                       ` (3 preceding siblings ...)
  2017-11-05 11:58     ` [PATCH 4/4] pager: make `pager.foo.command` imply `.enable=true` Martin Ågren
@ 2017-11-06 10:48     ` Jeff King
  2017-11-07 20:46       ` Martin Ågren
  4 siblings, 1 reply; 12+ messages in thread
From: Jeff King @ 2017-11-06 10:48 UTC (permalink / raw)
  To: Martin Ågren; +Cc: git, hkleynhans, Thomas Gummerer

On Sun, Nov 05, 2017 at 12:58:18PM +0100, Martin Ågren wrote:

> On 4 November 2017 at 10:28, Jeff King <peff@peff.net> wrote:
> >  - the pager.<cmd> config is mis-designed, because our config keys
> >    cannot represent all possible command names (e.g., case folding and
> >    illegal characters). This should be pager.<cmd>.enable or similar.
> >    Some discussion in (this message and the surrounding thread):
> >
> >      https://public-inbox.org/git/20170711101942.h2uwxtgzvgguzivu@sigill.intra.peff.net/
> >
> >    But I think you could find more by searching the archive.
> 
> I'm posting four patches I have on this to save others from redoing my
> work and findings. These patches feel a bit incomplete, which is why I
> put them to the side some time ago (and eventually forgot about them).

Thanks for sharing these; it's nice if we can avoid duplicating work.
We'll see if somebody at the event wants to pick up this topic.

> In particular, they do not teach `--paginate` to use the pager
> configured by `pager.foo.command`. It is already now possible to use
> `pager.foo` to say "I don't want you to page, but if I later give you
> `pager.foo=true`, this is the pager I want you to use". That does not
> work with `--paginate`, but this can all be explained -- indeed, we
> document that `--paginate` overrules `pager.foo`.

Hmm. I think the current behavior is actually buggy. I assume the
documentation you mean is from git.txt:

  --paginate::
          Pipe all output into 'less' (or if set, $PAGER) if standard
          output is a terminal.  This overrides the `pager.<cmd>`
          configuration options (see the "Configuration Mechanism" section
          below).

That comes from 06300d9753 (git.1: Clarify the behavior of the
--paginate option, 2010-02-14). But I think that commit was just trying
to clarify that "--paginate" overrides the defaults and config, but not
does say "always paginate".

I suspect nobody really noticed it in practice because once you've
configured "pager.foo", there's basically no need to ever use
"--paginate".

> If we teach `--paginate` to respect `pager.foo.command`, it seems that
> we would either 1) introduce a small (and possibly hard to understand
> and explain) difference between the old-style and the new-style
> pager-configuration or 2) knowingly change the behavior of `--paginate`
> with `pager.foo` or 3) knowingly change the behavior of
> `pager.foo=false` as documented in the first patch.

I think I'm suggesting (2), then.

-Peff

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Git Open Source Weekend London 11th/12th November
       [not found] <5A0036F7026201F600390334_0_28211@msllnjpmsgsv06>
@ 2017-11-07 19:05 ` Patrick Steinhardt
  0 siblings, 0 replies; 12+ messages in thread
From: Patrick Steinhardt @ 2017-11-07 19:05 UTC (permalink / raw)
  To: Henry Kleynhans; +Cc: t.gummerer, git, peff

[-- Attachment #1: Type: text/plain, Size: 3649 bytes --]

> Hi Patrick,
> 
> libgit2 tasks are welcome!  Feel free to bring your ideas
> along.  Also, Peff could add some to the task list if they are
> appropriate for others to participate on.

That's nice. I don't want to populate the list with too much
libgit2 specific things, but we do have a summary at [1] which
highlights a few starters as well as some possibly bigger
projects. I'd be grateful to see that URL mentioned, together
with the fact that there are two core developers present which
are able and happy to help out.

Please let me know whether that's enough for you to work with.

Thanks
Patrick

[1]: https://github.com/libgit2/libgit2/blob/master/PROJECTS.md

> Look forward to seeing you at the event!
> 
> Kind regards, Henry
> 
> From: ps@pks.im At: 11/05/17 12:42:53
> To: t.gummerer@gmail.com
> Cc: Henry Kleynhans (BLOOMBERG/ LONDON), git@vger.kernel.org, peff@peff.net
> Subject: Re: Git Open Source Weekend London 11th/12th November
> 
> On Wed, Nov 01, 2017 at 04:36:24PM +0000, Thomas Gummerer wrote:
> > Hi,
> > 
> > Bloomberg is hosting an Open Source Weekend in London on the 11th
> > & 12th November 2017 to contribute to the Git project.  We have
> > also confirmed that Peff will be amongst the mentors on hand to
> > guide attendees through their efforts!
> > 
> > Some of you may notice that we tried to organize this earlier in the
> > year, but unfortunately had to postpone it.  For this time around we
> > are further along in planning, and it's definitely happening :)
> > 
> > For those unfamiliar with Bloomberg Open Source weekends: These
> > events provide a great way for those who love working on code to
> > give back to the community. Come and help make difference to a
> > great project!
> > 
> > There will be tasks provided by the mentors, or bring your own if
> > you already have a great idea.  Also if you can't attend the weekend
> > and can think of a project which you would like tackled at this
> > event please let me know.  Obviously the projects should be
> > completable inside a weekend.
> > 
> > Normally attendees work in small groups on a specific task to
> > prevent anyone from getting stuck. Per usual, Bloomberg will
> > provide the venue, mentors, snacks and drinks.  Bring your
> > enthusiasm (and your laptop!) and come share in the fun!  The
> > event is also open to everyone, so feel free to pass on the
> > invite!
> > 
> > The event is free of charge, but please ensure that you are able
> > to attend the event before registering.  That will greatly help
> > us with accurate numbers for catering so that we don't create
> > unwanted waste!
> > 
> > You can register for the event here:
> > 
> > https://go.bloomberg.com/attend/invite/git-sprint-hosted-bloomberg/
> > 
> > Whether you already are a contributor (as probably most people on
> > this list are) or interested in starting to contribute to git or
> > have some friends that you'd like to get to contribute to git, it
> > would be great to see you at the event.
> > 
> > If you have any further questions feel free to get in touch.
> > 
> > - Thomas
> 
> Hi,
> 
> nice to hear, thanks for organizing. Due to having moved to
> London just that week I'd love to take part.
> 
> As I'm a core contributor of libgit2, I am interested in bringing
> forward libgit2 at that occasion. Would that be welcome or should
> participants keep strictly to the core git project? Just asking
> as I'd like to take part but have a rather long list of topics
> for libgit2 on my backlog which I'd like to tackle myself.
> 
> Regards
> Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable}
  2017-11-06 10:48     ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Jeff King
@ 2017-11-07 20:46       ` Martin Ågren
  0 siblings, 0 replies; 12+ messages in thread
From: Martin Ågren @ 2017-11-07 20:46 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List, hkleynhans, Thomas Gummerer

On 6 November 2017 at 11:48, Jeff King <peff@peff.net> wrote:
> On Sun, Nov 05, 2017 at 12:58:18PM +0100, Martin Ågren wrote:
>> In particular, they do not teach `--paginate` to use the pager
>> configured by `pager.foo.command`. It is already now possible to use
>> `pager.foo` to say "I don't want you to page, but if I later give you
>> `pager.foo=true`, this is the pager I want you to use". That does not
>> work with `--paginate`, but this can all be explained -- indeed, we
>> document that `--paginate` overrules `pager.foo`.
>
> Hmm. I think the current behavior is actually buggy. I assume the
> documentation you mean is from git.txt:
>
>   --paginate::
>           Pipe all output into 'less' (or if set, $PAGER) if standard
>           output is a terminal.  This overrides the `pager.<cmd>`
>           configuration options (see the "Configuration Mechanism" section
>           below).

Yes, that's what I meant.

> That comes from 06300d9753 (git.1: Clarify the behavior of the
> --paginate option, 2010-02-14). But I think that commit was just trying
> to clarify that "--paginate" overrides the defaults and config, but not
> does say "always paginate".

Thanks for digging.

You're probably right that this was not intended. I think I put way too
much weight on the "s" in "options". I understood it as "both ways
pager.foo can be used". From the commit message you point to, it seems
that only the boolean-ness was intended. (The "s" probably refers to
pager.foo, pager.bar, ...)

>> If we teach `--paginate` to respect `pager.foo.command`, it seems that
>> we would either 1) introduce a small (and possibly hard to understand
>> and explain) difference between the old-style and the new-style
>> pager-configuration or 2) knowingly change the behavior of
>> `--paginate`
>> with `pager.foo` or 3) knowingly change the behavior of
>> `pager.foo=false` as documented in the first patch.

> I think I'm suggesting (2), then.

I think you're right. If this topic doesn't get picked up in the near
future (in London or elsewhere), I'll get back to this.

A final thought for whoever tackles this: Playing with something like
`git -c pager.log=$foo --paginate log` vs `git -c core.pager=$foo
--paginate log` might be helpful. Possibly $foo=cat could be
particularly interesting, I haven't thought hard about it.

Martin

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-11-07 20:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-01 16:36 Git Open Source Weekend London 11th/12th November Thomas Gummerer
2017-11-04  9:28 ` Jeff King
2017-11-04 17:15   ` Philip Oakley
2017-11-05 11:58   ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Martin Ågren
2017-11-05 11:58     ` [PATCH 1/4] t7006: document that `pager.foo` can be partially preserved Martin Ågren
2017-11-05 11:58     ` [PATCH 2/4] pager: refactor `pager_command_config()` Martin Ågren
2017-11-05 11:58     ` [PATCH 3/4] pager: introduce `pager.*.command` and `.enable` Martin Ågren
2017-11-05 11:58     ` [PATCH 4/4] pager: make `pager.foo.command` imply `.enable=true` Martin Ågren
2017-11-06 10:48     ` [PATCH/DONOTAPPLY 0/4] first steps towards pager.foo.{command,enable} Jeff King
2017-11-07 20:46       ` Martin Ågren
2017-11-05 12:42 ` Git Open Source Weekend London 11th/12th November Patrick Steinhardt
     [not found] <5A0036F7026201F600390334_0_28211@msllnjpmsgsv06>
2017-11-07 19:05 ` Patrick Steinhardt

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).