All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
To: "Kyle J. McKay" <mackyle@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH 1/3] rebase: avoid non-function use of "return" on FreeBSD
Date: Sat, 12 Apr 2014 19:07:28 +0200	[thread overview]
Message-ID: <vpq38hi8oj3.fsf@anie.imag.fr> (raw)
In-Reply-To: <D3916156-0F84-4DD2-833F-64DF675325D3@gmail.com> (Kyle J. McKay's message of "Fri, 11 Apr 2014 16:08:53 -0700")

"Kyle J. McKay" <mackyle@gmail.com> writes:

> On Apr 11, 2014, at 10:30, Matthieu Moy wrote:
>> "Kyle J. McKay" <mackyle@gmail.com> writes:
>>
>>> There are already nested functions with file inclusion between both
>>> levels of nesting in git-rebase--interactive.sh and git-rebase--
>>> merge.sh now, so it's not introducing anything new.
>>
>> OK, so it's less serious than I thought. But still, we're
>> introducing a
>> function with 3 levels of nesting, split accross files, in an area
>> where
>> we know that at least one shell is buggy ...
>
> Currently in maint:
>
> The current code in maint does this:
>
> git-rebase.sh: top-level
>   git-rebase.sh: run_specific_rebase()
>     git-rebase.sh: run_specific_rebase_internal() -- contains "dot"
>       git-rebase--interactive.sh: top-level (using --continue or -- 
> skip)
>         git-rebase--interactive.sh: do_rest
>           git-rebase--interactive.sh: do_next

You're confusing function calls and function nesting. do_rest calls
do_next, but the definition of do_next is not nested within do_rest.

When I talk about nested function, I mean

f() {
	g() {
		...
	}
}

Obviously, having functions call each other is not an issue. That's what
functions are meant to be.

Now, having run_specific_rebase_internal include a file which defines
functions which contain nested functions _is_ something I find weird. It
both stresses the shell in a buggy area and makes the code harder to
understand.

> The problem with these changes, particularly the git-rebase-- 
> interactive.sh one is that a bunch of code is still run when the file
> is "dot" included.

Function definitions, and variables assignments. Is it so much of an
issue?

What's the difference between a function definition or variable
assignment within git-rebase--*.sh and within git-rebase.sh?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

  reply	other threads:[~2014-04-12 17:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-11  8:28 [PATCH 0/3] Fix support for FreeBSD's /bin/sh Kyle J. McKay
2014-04-11  8:28 ` [PATCH 1/3] rebase: avoid non-function use of "return" on FreeBSD Kyle J. McKay
2014-04-11  8:48   ` Matthieu Moy
2014-04-11 14:29     ` Kyle J. McKay
2014-04-11 17:30       ` Matthieu Moy
2014-04-11 23:08         ` Kyle J. McKay
2014-04-12 17:07           ` Matthieu Moy [this message]
2014-04-13  2:45             ` Kyle J. McKay
2014-04-14  8:24               ` Matthieu Moy
2014-04-14 22:28                 ` Junio C Hamano
2014-04-14 22:51   ` Junio C Hamano
2014-04-16  4:32     ` Kyle J. McKay
2014-04-16 16:47       ` Junio C Hamano
2014-04-16 18:11         ` Junio C Hamano
2014-04-16 18:23           ` Junio C Hamano
2014-04-17  0:41           ` Kyle J. McKay
2014-04-17 17:15             ` Junio C Hamano
2014-04-18  0:26               ` Kyle J. McKay
2014-04-11  8:28 ` [PATCH 2/3] Revert "rebase: fix run_specific_rebase's use of "return" on FreeBSD" Kyle J. McKay
2014-04-11  8:28 ` [PATCH 3/3] test: fix t5560 on FreeBSD Kyle J. McKay
2014-04-11 20:52   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=vpq38hi8oj3.fsf@anie.imag.fr \
    --to=matthieu.moy@grenoble-inp.fr \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mackyle@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.