From f6bc1a54cfe9da3a7d2124915d621676bb9271a9 Mon Sep 17 00:00:00 2001 From: kasemir Date: Fri, 12 Jun 2026 09:36:05 -0400 Subject: [PATCH] Fix misleading "not fully resolved" macro warnings Improve macro check so that `LongStringRecord.VAL$` is no longer mistaken for an unresolved macro. Generally turn WARNING into INFO because WARNING tends to mislead end users as they're always issued from the editor and might indicate problems in runtime that doesn't really matter. INFO messages can still be used to trace down macro problems. --- .../builder/model/MacroizedWidgetProperty.java | 6 +++--- .../phoebus/framework/macros/MacroHandler.java | 9 +++++++-- .../org/phoebus/framework/macros/Macros.java | 4 ++-- .../phoebus/framework/macros/MacrosUnitTest.java | 16 +++++++++++++++- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/app/display/model/src/main/java/org/csstudio/display/builder/model/MacroizedWidgetProperty.java b/app/display/model/src/main/java/org/csstudio/display/builder/model/MacroizedWidgetProperty.java index 6959ec1b87..47dd0ef223 100644 --- a/app/display/model/src/main/java/org/csstudio/display/builder/model/MacroizedWidgetProperty.java +++ b/app/display/model/src/main/java/org/csstudio/display/builder/model/MacroizedWidgetProperty.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015-2020 Oak Ridge National Laboratory. + * Copyright (c) 2015-2026 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -157,13 +157,13 @@ public synchronized T getValue() expanded = specification; } - // Warn if expanded text still contains macros. + // Log if expanded text still contains macros. // .. unless specification contained escaped macros, // which have been un-escaped. // Note that this ignores remaining macros as soon // as there is just one escaped macro, as in "$(MISSING_AND_IGNORED) \\$(ESCAPED)" if (MacroHandler.containsMacros(expanded) && ! specification.contains("\\$")) - logger.log(Level.WARNING, widget + " '" + getName() + "' is not fully resolved: '" + expanded + "'"); + logger.log(Level.INFO, widget + " '" + getName() + "' is not fully resolved: '" + expanded + "'"); try { diff --git a/core/framework/src/main/java/org/phoebus/framework/macros/MacroHandler.java b/core/framework/src/main/java/org/phoebus/framework/macros/MacroHandler.java index 52dee8f8fa..bf9190748a 100644 --- a/core/framework/src/main/java/org/phoebus/framework/macros/MacroHandler.java +++ b/core/framework/src/main/java/org/phoebus/framework/macros/MacroHandler.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015-2019 Oak Ridge National Laboratory. + * Copyright (c) 2015-2026 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -27,7 +27,12 @@ public class MacroHandler */ public static boolean containsMacros(final String input) { - return input != null && input.indexOf('$') >= 0; + // A complete check for unresolved macros, including multi-level macros, + // requires a MacroValueProvider. + // This, however, could be called from an editor where no macros are available. + // Implementation thus limited to a simple check for potential macros + return input != null && + (input.indexOf("$(") >= 0 || input.indexOf("${") >= 0); } /** Replace macros in input diff --git a/core/framework/src/main/java/org/phoebus/framework/macros/Macros.java b/core/framework/src/main/java/org/phoebus/framework/macros/Macros.java index f71f041827..4338aa5532 100644 --- a/core/framework/src/main/java/org/phoebus/framework/macros/Macros.java +++ b/core/framework/src/main/java/org/phoebus/framework/macros/Macros.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015-2023 Oak Ridge National Laboratory. + * Copyright (c) 2015-2026 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -260,7 +260,7 @@ public void expandValues(final Macros base) // Not fatal, in fact common when creating displays, // but log with exception to get stack trace in case // origin of macro needs to be debugged - logger.log(Level.WARNING, "Incomplete macro expansion " + name + "='" + expanded + "'", + logger.log(Level.INFO, "Incomplete macro expansion " + name + "='" + expanded + "'", new Exception("Macro spec " + name + "='" + spec + "' does not fully resolve")); } } diff --git a/core/framework/src/test/java/org/phoebus/framework/macros/MacrosUnitTest.java b/core/framework/src/test/java/org/phoebus/framework/macros/MacrosUnitTest.java index b513a3fc6d..fdfe9451d9 100644 --- a/core/framework/src/test/java/org/phoebus/framework/macros/MacrosUnitTest.java +++ b/core/framework/src/test/java/org/phoebus/framework/macros/MacrosUnitTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2015-2023 Oak Ridge National Laboratory. + * Copyright (c) 2015-2026 Oak Ridge National Laboratory. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -42,11 +42,25 @@ public void testNames() @Test public void testCheck() { + // Clearly not a macro assertThat(MacroHandler.containsMacros("Plain Text"), equalTo(false)); + + // Clearly a macro assertThat(MacroHandler.containsMacros("${S}"), equalTo(true)); assertThat(MacroHandler.containsMacros("This is $(S)"), equalTo(true)); assertThat(MacroHandler.containsMacros("$(MACRO)"), equalTo(true)); + + // Two macro levels assertThat(MacroHandler.containsMacros("$(${MACRO})"), equalTo(true)); + + // Not a macro + assertThat(MacroHandler.containsMacros("StringPV.VAL$"), equalTo(false)); + assertThat(MacroHandler.containsMacros("StringPV.$"), equalTo(false)); + + // Debatable: + // Would a first macro expansion pass turn '\\$(S)' into '$(S)', + // which then _is_ a macro that's expanded in a second pass? + // We simply check for '$(' and take that as a macro assertThat(MacroHandler.containsMacros("Escaped \\$(S)"), equalTo(true)); assertThat(MacroHandler.containsMacros("Escaped \\$(S) Used $(S)"), equalTo(true)); }