Skip to content

Commit 736ab8d

Browse files
authored
Merge pull request #3831 from ControlSystemStudio/macro_warning
Fix misleading "not fully resolved" macro warnings
2 parents 247236d + f6bc1a5 commit 736ab8d

4 files changed

Lines changed: 27 additions & 8 deletions

File tree

app/display/model/src/main/java/org/csstudio/display/builder/model/MacroizedWidgetProperty.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2015-2020 Oak Ridge National Laboratory.
2+
* Copyright (c) 2015-2026 Oak Ridge National Laboratory.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -157,13 +157,13 @@ public synchronized T getValue()
157157
expanded = specification;
158158
}
159159

160-
// Warn if expanded text still contains macros.
160+
// Log if expanded text still contains macros.
161161
// .. unless specification contained escaped macros,
162162
// which have been un-escaped.
163163
// Note that this ignores remaining macros as soon
164164
// as there is just one escaped macro, as in "$(MISSING_AND_IGNORED) \\$(ESCAPED)"
165165
if (MacroHandler.containsMacros(expanded) && ! specification.contains("\\$"))
166-
logger.log(Level.WARNING, widget + " '" + getName() + "' is not fully resolved: '" + expanded + "'");
166+
logger.log(Level.INFO, widget + " '" + getName() + "' is not fully resolved: '" + expanded + "'");
167167

168168
try
169169
{

core/framework/src/main/java/org/phoebus/framework/macros/MacroHandler.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2015-2019 Oak Ridge National Laboratory.
2+
* Copyright (c) 2015-2026 Oak Ridge National Laboratory.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -27,7 +27,12 @@ public class MacroHandler
2727
*/
2828
public static boolean containsMacros(final String input)
2929
{
30-
return input != null && input.indexOf('$') >= 0;
30+
// A complete check for unresolved macros, including multi-level macros,
31+
// requires a MacroValueProvider.
32+
// This, however, could be called from an editor where no macros are available.
33+
// Implementation thus limited to a simple check for potential macros
34+
return input != null &&
35+
(input.indexOf("$(") >= 0 || input.indexOf("${") >= 0);
3136
}
3237

3338
/** Replace macros in input

core/framework/src/main/java/org/phoebus/framework/macros/Macros.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2015-2023 Oak Ridge National Laboratory.
2+
* Copyright (c) 2015-2026 Oak Ridge National Laboratory.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -260,7 +260,7 @@ public void expandValues(final Macros base)
260260
// Not fatal, in fact common when creating displays,
261261
// but log with exception to get stack trace in case
262262
// origin of macro needs to be debugged
263-
logger.log(Level.WARNING, "Incomplete macro expansion " + name + "='" + expanded + "'",
263+
logger.log(Level.INFO, "Incomplete macro expansion " + name + "='" + expanded + "'",
264264
new Exception("Macro spec " + name + "='" + spec + "' does not fully resolve"));
265265
}
266266
}

core/framework/src/test/java/org/phoebus/framework/macros/MacrosUnitTest.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2015-2023 Oak Ridge National Laboratory.
2+
* Copyright (c) 2015-2026 Oak Ridge National Laboratory.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -42,11 +42,25 @@ public void testNames()
4242
@Test
4343
public void testCheck()
4444
{
45+
// Clearly not a macro
4546
assertThat(MacroHandler.containsMacros("Plain Text"), equalTo(false));
47+
48+
// Clearly a macro
4649
assertThat(MacroHandler.containsMacros("${S}"), equalTo(true));
4750
assertThat(MacroHandler.containsMacros("This is $(S)"), equalTo(true));
4851
assertThat(MacroHandler.containsMacros("$(MACRO)"), equalTo(true));
52+
53+
// Two macro levels
4954
assertThat(MacroHandler.containsMacros("$(${MACRO})"), equalTo(true));
55+
56+
// Not a macro
57+
assertThat(MacroHandler.containsMacros("StringPV.VAL$"), equalTo(false));
58+
assertThat(MacroHandler.containsMacros("StringPV.$"), equalTo(false));
59+
60+
// Debatable:
61+
// Would a first macro expansion pass turn '\\$(S)' into '$(S)',
62+
// which then _is_ a macro that's expanded in a second pass?
63+
// We simply check for '$(' and take that as a macro
5064
assertThat(MacroHandler.containsMacros("Escaped \\$(S)"), equalTo(true));
5165
assertThat(MacroHandler.containsMacros("Escaped \\$(S) Used $(S)"), equalTo(true));
5266
}

0 commit comments

Comments
 (0)