fix(sub): omit hyphen for empty remark variables (#6101)

* fix(sub): omit hyphen for empty remark variables

The default INBOUND-EMAIL template left a leading hyphen when an inbound had no remark after display remarks became template-driven in b0c1156dd. Treat a hyphen between adjacent variables as their separator and drop it when it would lead the output or when the value after it is empty, so an empty variable in the middle of a template still leaves a single separator between its neighbours. Literal leading hyphens written into the template are preserved.

* fix(sub): elide the remark separator after leading decoration

The separator between two adjacent tokens was kept as soon as any text had
reached the segment, so a template opening with decoration still rendered
"🌐-john" for an inbound with no remark. Track whether a token has produced
a value rather than testing the accumulated output, so the hyphen is elided
for any prefix that carries no token value of its own, and the builder is no
longer rescanned once per token.

internal/sub/remark_vars.go

@@ -40,6 +40,26 @@ func (ctx remarkContext) configName() string {
 // underscores only, so ordinary braces in a remark are left untouched.
 var remarkVarRe = regexp.MustCompile(`\{\{([A-Z_]+)\}\}`)
 
+// remarkToken is one {{TOKEN}} occurrence: its name and the byte range it spans
+// in the segment it was found in.
+type remarkToken struct {
+	name  string
+	start int
+	end   int
+}
+
+// remarkTokens locates every {{TOKEN}} in seg. Both the template-level filter and
+// the value-level expansion walk a segment through this, so they share one notion
+// of where a token begins and ends and what the literal text between two of them is.
+func remarkTokens(seg string) []remarkToken {
+	locs := remarkVarRe.FindAllStringSubmatchIndex(seg, -1)
+	tokens := make([]remarkToken, len(locs))
+	for i, loc := range locs {
+		tokens[i] = remarkToken{name: seg[loc[2]:loc[3]], start: loc[0], end: loc[1]}
+	}
+	return tokens
+}
+
 // unlimitedMark is the value the human-readable quota/expiry tokens render when
 // the client has no limit. A segment built only around such a token carries no
 // information, so it is dropped rather than printed as "∞" (see expandRemarkVars).
@@ -107,7 +127,9 @@ func translateUISingleBrackets(template string) string {
 // value. Unknown tokens resolve to "" (never the literal text). The template is
 // split on "|" into segments: a segment whose only value is an unlimited quota
 // or expiry (∞) drops out whole — decoration and separator included — so an
-// unlimited client gets "host" instead of "host|📊∞|⏳∞D".
+// unlimited client gets "host" instead of "host|📊∞|⏳∞D". Inside a surviving
+// segment expandSegment also elides a hyphen separator an empty token would
+// leave dangling.
 func expandRemarkVars(template string, ctx remarkContext) string {
 	template = translateUISingleBrackets(template)
 	if !strings.Contains(template, "{{") {
@@ -129,18 +151,43 @@ func expandRemarkVars(template string, ctx remarkContext) string {
 // — so it leaves no stray "|" separator or dangling decoration. A segment mixing,
 // say, {{EMAIL}} with {{TRAFFIC_LEFT}} is kept, and a pure-literal segment (no
 // tokens) is always kept.
+//
+// A hyphen standing alone between two adjacent tokens is treated as their
+// separator and elided when no token before it has produced a value yet or when
+// the token after it resolves to nothing. "{{INBOUND}}-{{EMAIL}}" gives "john"
+// for an inbound with no remark, "🌐{{INBOUND}}-{{EMAIL}}" gives "🌐john" so
+// leading decoration does not keep the separator alive, and
+// "{{EMAIL}}-{{INBOUND}}-{{EMAIL}}" keeps a single separator when the middle
+// token is empty. A hyphen anywhere else in the segment is literal text and is
+// kept as written.
 func expandSegment(seg string, ctx remarkContext) (string, bool) {
-	hasToken, hasOtherValue := false, false
-	out := remarkVarRe.ReplaceAllStringFunc(seg, func(m string) string {
-		hasToken = true
-		token := m[2 : len(m)-2]
-		val := remarkVarValue(token, ctx)
-		if val != "" && (!unlimitedDropTokens[token] || val != unlimitedMark) {
+	tokens := remarkTokens(seg)
+	hasToken, hasOtherValue := len(tokens) > 0, false
+	values := make([]string, len(tokens))
+	for i, tok := range tokens {
+		val := remarkVarValue(tok.name, ctx)
+		values[i] = val
+		if val != "" && (!unlimitedDropTokens[tok.name] || val != unlimitedMark) {
 			hasOtherValue = true
 		}
-		return val
-	})
-	return out, hasToken && !hasOtherValue
+	}
+
+	var result strings.Builder
+	start, wroteValue := 0, false
+	for i, tok := range tokens {
+		result.WriteString(seg[start:tok.start])
+		result.WriteString(values[i])
+		wroteValue = wroteValue || values[i] != ""
+		start = tok.end
+		if i+1 < len(tokens) {
+			between := seg[start:tokens[i+1].start]
+			if strings.TrimSpace(between) == "-" && (!wroteValue || values[i+1] == "") {
+				start = tokens[i+1].start
+			}
+		}
+	}
+	result.WriteString(seg[start:])
+	return result.String(), hasToken && !hasOtherValue
 }
 
 func remarkVarValue(token string, ctx remarkContext) string {
@@ -511,11 +558,17 @@ func filterRemarkTemplate(template string, remove map[string]bool) string {
 	return strings.Join(kept, "|")
 }
 
+// filterRemarkSegment drops whole token categories from one segment while it is
+// still a template, before any value is known. Literal text touching a removed
+// token goes with it and the surviving runs rejoin with a space, so filtering the
+// usage tokens out of "{{EMAIL}} 📊{{TRAFFIC_LEFT}}" leaves "{{EMAIL}}". This is
+// the template-level counterpart to expandSegment, which works one layer later on
+// tokens that survive here but resolve to an empty value.
 func filterRemarkSegment(seg string, remove map[string]bool) string {
-	locs := remarkVarRe.FindAllStringSubmatchIndex(seg, -1)
+	tokens := remarkTokens(seg)
 	hasRemove := false
-	for _, loc := range locs {
-		if remove[seg[loc[2]:loc[3]]] {
+	for _, tok := range tokens {
+		if remove[tok.name] {
 			hasRemove = true
 			break
 		}
@@ -525,28 +578,28 @@ func filterRemarkSegment(seg string, remove map[string]bool) string {
 	}
 	runs := make([]string, 0, 2)
 	runStart, leftRemoved := 0, false
-	for _, loc := range locs {
-		if !remove[seg[loc[2]:loc[3]]] {
+	for _, tok := range tokens {
+		if !remove[tok.name] {
 			continue
 		}
-		runs = appendKeptRun(runs, seg[runStart:loc[0]], leftRemoved, true)
-		runStart, leftRemoved = loc[1], true
+		runs = appendKeptRun(runs, seg[runStart:tok.start], leftRemoved, true)
+		runStart, leftRemoved = tok.end, true
 	}
 	runs = appendKeptRun(runs, seg[runStart:], leftRemoved, false)
 	return strings.Join(runs, " ")
 }
 
 func appendKeptRun(runs []string, run string, leftRemoved, rightRemoved bool) []string {
-	locs := remarkVarRe.FindAllStringSubmatchIndex(run, -1)
-	if len(locs) == 0 {
+	tokens := remarkTokens(run)
+	if len(tokens) == 0 {
 		return runs
 	}
 	start, end := 0, len(run)
 	if leftRemoved {
-		start = locs[0][0]
+		start = tokens[0].start
 	}
 	if rightRemoved {
-		end = locs[len(locs)-1][1]
+		end = tokens[len(tokens)-1].end
 	}
 	if frag := strings.TrimSpace(run[start:end]); frag != "" {
 		runs = append(runs, frag)

internal/sub/remark_vars_test.go

@@ -102,6 +102,41 @@ func TestExpandRemarkVars_EdgeCases(t *testing.T) {
 	}
 }
 
+// defaultRemarkTemplate mirrors the panel's shipped remark template, the one an
+// inbound with no remark used to render with a leading hyphen.
+const defaultRemarkTemplate = "{{INBOUND}}-{{EMAIL}}|📊{{TRAFFIC_LEFT}}|⏳{{DAYS_LEFT}}D"
+
+func TestExpandRemarkVars_DropsHyphenBetweenEmptyTokens(t *testing.T) {
+	cases := []struct {
+		name    string
+		tmpl    string
+		inbound string
+		email   string
+		want    string
+	}{
+		{name: "both values", tmpl: "{{INBOUND}}-{{EMAIL}}", inbound: "Germany", email: "john", want: "Germany-john"},
+		{name: "empty inbound", tmpl: "{{INBOUND}}-{{EMAIL}}", email: "john", want: "john"},
+		{name: "empty email", tmpl: "{{INBOUND}} - {{EMAIL}}", inbound: "Germany", want: "Germany"},
+		{name: "literal leading hyphen", tmpl: "-{{EMAIL}}", email: "john", want: "-john"},
+		{name: "literal leading hyphen before an empty token", tmpl: "-{{INBOUND}}-{{EMAIL}}", email: "john", want: "-john"},
+		{name: "empty var between two values keeps one separator", tmpl: "{{EMAIL}}-{{INBOUND}}-{{EMAIL}}", email: "john", want: "john-john"},
+		{name: "emoji decoration before an empty token", tmpl: "🌐{{INBOUND}}-{{EMAIL}}", email: "john", want: "🌐john"},
+		{name: "literal word before an empty token", tmpl: "Sub {{INBOUND}}-{{EMAIL}}", email: "john", want: "Sub john"},
+		{name: "decoration kept when both tokens resolve", tmpl: "🌐{{INBOUND}}-{{EMAIL}}", inbound: "Germany", email: "john", want: "🌐Germany-john"},
+		{name: "default template, empty inbound", tmpl: defaultRemarkTemplate, email: "john", want: "john"},
+		{name: "default template, empty email", tmpl: defaultRemarkTemplate, inbound: "Germany", want: "Germany"},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := expandCtx(model.Client{Email: tt.email}, xray.ClientTraffic{Enable: true}, &model.Inbound{Remark: tt.inbound})
+			if got := expandRemarkVars(tt.tmpl, ctx); got != tt.want {
+				t.Errorf("expandRemarkVars(%q) = %q, want %q", tt.tmpl, got, tt.want)
+			}
+		})
+	}
+}
+
 // An unlimited client drops the quota/expiry segments whole — decoration and the
 // "|" separator included — instead of printing "📊∞|⏳∞D".
 func TestExpandRemarkVars_DropUnlimitedSegments(t *testing.T) {