Re: [PATCH 1/3] rebase: avoid non-function use of "return" on FreeBSD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Apr 11, 2014, at 01:48, Matthieu Moy wrote:
"Kyle J. McKay" <mackyle@xxxxxxxxx> writes:

If script2.sh is changed to this:

# script2.sh fixed
main() {
       if [ 5 -gt 3 ]; then
               return 5
       fi
       case bad in *)
               echo always shows
       esac
       echo should not get here
       ! :
}
main

Wouldn't it be better to just stop using . within function?

The .-ed script could define the complete function, and then the
function would be used from the toplevel script.

If I understand correctly, your version uses nested functions with file inclusion between both levels of nesting. That might work for the shells
you tested, but if the goal is to avoid using tricky features that may
trigger bugs on some shells, that seems backward.

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.

IOW, why not move the whole run_specific_rebase_internal function to
git-rebase--$type?

So what change are you proposing exactly?

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--$type.sh: top-level -- has "return"

To make the kind of change I think you're proposing would be somewhat more invasive than the proposed patch. Each of the git-rebase--$type scripts would have to be modified not to do anything other than define functions when included which would require some code movement to avoid having two "main" functions -- either that or there need to be multiple "dot" includes in git-rebase.sh so the code not in a function only executes when the selected case that uses it is active -- or the entire contents of each git-rebase--$type script gets indented and becomes a function definition, but that would introduce functions defining functions which seems like it would add use of a new tricky feature not previously used.

I'm not saying it's a bad idea, it's just somewhat more invasive than simply inserting 3 lines.

--Kyle
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]