pkg/terminal: allow postfix if for breakpoint conds (#3693)

Allows for a user to specify the breakpoint condition directly
when creating the breakpoint. The new syntax looks like the
following:

```
break <name> <locspec> [if <expression>]
```

Also updates docs to include more examples and locspec description
instead of directing users to the online / source documentation.

Documentation/cli/README.md

@@ -108,9 +108,32 @@ If regex is specified only function arguments with a name matching it will be re
 ## break
 Sets a breakpoint.
 
-	break [name] [locspec]
+	break [name] [locspec] [if <condition>]
 
-See [Documentation/cli/locspec.md](//github.com/go-delve/delve/tree/master/Documentation/cli/locspec.md) for the syntax of locspec. If locspec is omitted a breakpoint will be set on the current line.
+Locspec is a location specifier in the form of:
+
+  * *<address> Specifies the location of memory address address. address can be specified as a decimal, hexadecimal or octal number
+  * <filename>:<line> Specifies the line line in filename. filename can be the partial path to a file or even just the base name as long as the expression remains unambiguous.
+  * <line> Specifies the line line in the current file
+  * +<offset> Specifies the line offset lines after the current one
+  * -<offset> Specifies the line offset lines before the current one
+  * <function>[:<line>] Specifies the line line inside function. 
+      The full syntax for function is <package>.(*<receiver type>).<function name> however the only required element is the function name,
+      everything else can be omitted as long as the expression remains unambiguous. For setting a breakpoint on an init function (ex: main.init),
+      the <filename>:<line> syntax should be used to break in the correct init function at the correct location.
+  * /<regex>/ Specifies the location of all the functions matching regex
+
+If locspec is omitted a breakpoint will be set on the current line.
+
+If you would like to assign a name to the breakpoint you can do so with the form:
+
+  break mybpname main.go:4
+
+Finally, you can assign a condition to the newly created breakpoint by using the 'if' postfix form, like so:
+
+  break main.go:55 if i == 5
+
+Alternatively you can set a condition on a breakpoint after created by using the 'on' command.
 
 See also: "help on", "help cond" and "help clear"
 

_fixtures/test if path/main.go

@@ -0,0 +1,5 @@
+package main
+
+func main() {
+	println("here")
+}

pkg/locspec/locations.go

@@ -361,7 +361,6 @@ func (ale AmbiguousLocationError) Error() string {
 		for i := range ale.CandidatesLocation {
 			candidates = append(candidates, ale.CandidatesLocation[i].Function.Name())
 		}
-
 	} else {
 		candidates = ale.CandidatesString
 	}

pkg/terminal/command.go

@@ -122,9 +122,32 @@ func DebugCommands(client service.Client) *Commands {
 Type "help" followed by the name of a command for more information about it.`},
 		{aliases: []string{"break", "b"}, group: breakCmds, cmdFn: breakpoint, helpMsg: `Sets a breakpoint.
 
-	break [name] [locspec]
+	break [name] [locspec] [if <condition>]
 
-See Documentation/cli/locspec.md for the syntax of locspec. If locspec is omitted a breakpoint will be set on the current line.
+Locspec is a location specifier in the form of:
+
+  * *<address> Specifies the location of memory address address. address can be specified as a decimal, hexadecimal or octal number
+  * <filename>:<line> Specifies the line line in filename. filename can be the partial path to a file or even just the base name as long as the expression remains unambiguous.
+  * <line> Specifies the line line in the current file
+  * +<offset> Specifies the line offset lines after the current one
+  * -<offset> Specifies the line offset lines before the current one
+  * <function>[:<line>] Specifies the line line inside function. 
+      The full syntax for function is <package>.(*<receiver type>).<function name> however the only required element is the function name,
+      everything else can be omitted as long as the expression remains unambiguous. For setting a breakpoint on an init function (ex: main.init),
+      the <filename>:<line> syntax should be used to break in the correct init function at the correct location.
+  * /<regex>/ Specifies the location of all the functions matching regex
+
+If locspec is omitted a breakpoint will be set on the current line.
+
+If you would like to assign a name to the breakpoint you can do so with the form:
+
+  break mybpname main.go:4
+
+Finally, you can assign a condition to the newly created breakpoint by using the 'if' postfix form, like so:
+
+  break main.go:55 if i == 5
+
+Alternatively you can set a condition on a breakpoint after created by using the 'on' command.
 
 See also: "help on", "help cond" and "help clear"`},
 		{aliases: []string{"trace", "t"}, group: breakCmds, cmdFn: tracepoint, allowedPrefixes: onPrefix, helpMsg: `Set tracepoint.
@@ -1796,31 +1819,54 @@ func formatBreakpointAttrs(prefix string, bp *api.Breakpoint, includeTrace bool)
 }
 
 func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]*api.Breakpoint, error) {
-	args := config.Split2PartsBySpace(argstr)
+	var (
+		cond string
+		spec string
 
-	requestedBp := &api.Breakpoint{}
-	spec := ""
-	switch len(args) {
-	case 1:
-		if len(args[0]) != 0 {
-			spec = argstr
-		} else {
-			// no arg specified
-			spec = "+0"
+		requestedBp = &api.Breakpoint{}
+	)
+
+	parseSpec := func(args []string) error {
+		switch len(args) {
+		case 1:
+			if len(args[0]) != 0 {
+				spec = argstr
+			} else {
+				// no arg specified
+				spec = "+0"
+			}
+		case 2:
+			if api.ValidBreakpointName(args[0]) == nil {
+				requestedBp.Name = args[0]
+				spec = args[1]
+			} else {
+				spec = argstr
+			}
+		default:
+			return fmt.Errorf("address required")
 		}
-	case 2:
-		if api.ValidBreakpointName(args[0]) == nil {
-			requestedBp.Name = args[0]
-			spec = args[1]
-		} else {
-			spec = argstr
-		}
-	default:
-		return nil, fmt.Errorf("address required")
+		return nil
+	}
+
+	args := config.Split2PartsBySpace(argstr)
+	if err := parseSpec(args); err != nil {
+		return nil, err
 	}
 
 	requestedBp.Tracepoint = tracepoint
 	locs, substSpec, findLocErr := t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
+	if findLocErr != nil {
+		r := regexp.MustCompile(`^if | if `)
+		if match := r.FindStringIndex(argstr); match != nil {
+			cond = argstr[match[1]:]
+			argstr = argstr[:match[0]]
+			args = config.Split2PartsBySpace(argstr)
+			if err := parseSpec(args); err != nil {
+				return nil, err
+			}
+			locs, substSpec, findLocErr = t.client.FindLocation(ctx.Scope, spec, true, t.substitutePathRules())
+		}
+	}
 	if findLocErr != nil && requestedBp.Name != "" {
 		requestedBp.Name = ""
 		spec = argstr
@@ -1853,6 +1899,7 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
 		fmt.Fprintf(t.stdout, "%s set at %s\n", formatBreakpointName(bp, true), t.formatBreakpointLocation(bp))
 		return nil, nil
 	}
+
 	if findLocErr != nil {
 		return nil, findLocErr
 	}
@@ -1869,6 +1916,7 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
 			requestedBp.LoadArgs = &ShortLoadConfig
 		}
 
+		requestedBp.Cond = cond
 		bp, err := t.client.CreateBreakpointWithExpr(requestedBp, spec, t.substitutePathRules(), false)
 		if err != nil {
 			return nil, err
@@ -2423,7 +2471,6 @@ func parseStackArgs(argstr string) (stackArgs, error) {
 					return 0, fmt.Errorf("expected number after %s: %v", name, err)
 				}
 				return n, nil
-
 			}
 			switch args[i] {
 			case "-full":

pkg/terminal/command_test.go

@@ -1435,6 +1435,39 @@ func TestCreateBreakpointByLocExpr(t *testing.T) {
 	})
 }
 
+func TestCreateBreakpointWithCondition(t *testing.T) {
+	withTestTerminal("break", t, func(term *FakeTerminal) {
+		term.MustExec("break bp1 main.main:4 if i == 3")
+		listIsAt(t, term, "continue", 7, -1, -1)
+		out := term.MustExec("print i")
+		t.Logf("%q", out)
+		if !strings.Contains(out, "3\n") {
+			t.Fatalf("wrong value of i")
+		}
+	})
+}
+
+func TestCreateBreakpointWithCondition2(t *testing.T) {
+	withTestTerminal("break", t, func(term *FakeTerminal) {
+		term.MustExec("continue main.main:4")
+		term.MustExec("break if i == 3")
+		listIsAt(t, term, "continue", 7, -1, -1)
+		out := term.MustExec("print i")
+		t.Logf("%q", out)
+		if !strings.Contains(out, "3\n") {
+			t.Fatalf("wrong value of i")
+		}
+	})
+}
+
+func TestCreateBreakpointWithCondition3(t *testing.T) {
+	withTestTerminal("test if path/main", t, func(term *FakeTerminal) {
+		// We should not attempt to parse this as a condition.
+		term.MustExec(`break _fixtures/test if path/main.go:4`)
+		listIsAt(t, term, "continue", 4, -1, -1)
+	})
+}
+
 func TestRestartBreakpoints(t *testing.T) {
 	// Tests that breakpoints set using just a line number and with a line
 	// offset are preserved after restart. See issue #3423.