[PATCH v2] dispatch: refactor tortured debugger setup logic

[Yuya Nishihara]

On Tue, 05 Jan 2016 11:27:21 -0800, Bryan O'Sullivan wrote:
> # HG changeset patch
> # User Bryan O'Sullivan <bos at serpentine.com>
> # Date 1452022007 28800
> #      Tue Jan 05 11:26:47 2016 -0800
> # Node ID 82033eb9055b83fe7e6b1500dc893c3ca4adcf2a
> # Parent  f9a7e04fb6e1d31887c9a866fef852e52db7ca21
> dispatch: refactor tortured debugger setup logic
> 
> This patch was triggered by observing that the pdb debugger was
> always loaded regardless of whether a debugger was to be used.
> 
> The logic as it existed was a thicket of accidental complexity.
> It's now much easier to follow (and only loads a debugger if needed).
> 
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -139,14 +139,6 @@ def _runcatch(req):
>  
>      try:
>          try:
> -            debugger = 'pdb'
> -            debugtrace = {
> -                'pdb' : pdb.set_trace
> -            }
> -            debugmortem = {
> -                'pdb' : pdb.post_mortem
> -            }
> -
>              # read --config before doing anything else
>              # (e.g. to change trust settings for reading .hg/hgrc)
>              cfgs = _parseconfig(req.ui, _earlygetopt(['--config'], req.args))
> @@ -157,36 +149,29 @@ def _runcatch(req):
>                  for sec, name, val in cfgs:
>                      req.repo.ui.setconfig(sec, name, val, source='--config')
>  
> -            # developer config: ui.debugger
> -            debugger = ui.config("ui", "debugger")
> -            debugmod = pdb
> -            if not debugger or ui.plain():
> -                # if we are in HGPLAIN mode, then disable custom debugging
> +            if '--debugger' in req.args:
> +                # if we are in HGPLAIN mode, ignore custom debugging
>                  debugger = 'pdb'
> -            elif '--debugger' in req.args:
> +                if not ui.plain():
> +                    # developer config: ui.debugger
> +                    debugger = ui.config('ui', 'debugger', default='pdb')
> +
> +                debugmod = pdb
>                  # This import can be slow for fancy debuggers, so only
>                  # do it when absolutely necessary, i.e. when actual
>                  # debugging has been requested
>                  with demandimport.deactivated():
>                      try:
>                          debugmod = __import__(debugger)
> -                    except ImportError:
> -                        pass # Leave debugmod = pdb
> +                    except (ImportError, ValueError):
> +                        ui.warn(_("%r debugger specified "
> +                                  "but its module was not found\n") % debugger)
>  
> -            debugtrace[debugger] = debugmod.set_trace
> -            debugmortem[debugger] = debugmod.post_mortem
> +                    # enter the debugger before command execution
> +                    ui.warn(_("entering debugger - "
> +                              "type c to continue starting hg or h for help\n"))
>  
> -            # enter the debugger before command execution
> -            if '--debugger' in req.args:
> -                ui.warn(_("entering debugger - "
> -                        "type c to continue starting hg or h for help\n"))
> -
> -                if (debugger != 'pdb' and
> -                    debugtrace[debugger] == debugtrace['pdb']):
> -                    ui.warn(_("%s debugger specified "
> -                              "but its module was not found\n") % debugger)
> -                with demandimport.deactivated():
> -                    debugtrace[debugger]()
> +                    debugmod.set_trace()
>              try:
>                  return _dispatch(req)
>              finally:
> @@ -195,7 +180,7 @@ def _runcatch(req):
>              # enter the debugger when we hit an exception
>              if '--debugger' in req.args:
>                  traceback.print_exc()
> -                debugmortem[debugger](sys.exc_info()[2])
> +                debugmod.post_mortem(sys.exc_info()[2])

Still it has unbound debugmod issue for "--debugger --config foo=bar=baz".

Perhaps we could do

  debugmod = None
  try:
      ...
      if '--debugger' in req.args:
          debugmod = pdb or something
  except:
      if debugmod:  # in place of '--debugger' in req.args
          debugmod.post_mortem(...)