Conversation
Implemented the TAG_PAD configuration
joakime
left a comment
There was a problem hiding this comment.
Not quite right.
Naming of this configuration is the big one for me.
The implementation could be simpler too.
And there's no need for @deprecated here (we are still in alpha/beta phase)
| */ | ||
| static final String NAME_CONDENSE_KEY = "org.eclipse.jetty.logging.appender.NAME_CONDENSE"; | ||
| static final String THREAD_PADDING_KEY = "org.eclipse.jetty.logging.appender.THREAD_PADDING"; | ||
| static final String TAG_PAD_KEY = "org.eclipse.jetty.logging.appender.TAG_PAD"; |
There was a problem hiding this comment.
This needs a better name.
TAG is meaningless and vague.
THREAD isn't what this is about either.
There was a problem hiding this comment.
If this were instead a name that indicated the tab stop of where the "message" portion of the log output should begin, then the logic below gets simpler.
Example
package text;
public class TabStopDemo
{
public static void main(String[] args)
{
String padding = " ".repeat(20);
System.out.printf("[%s]%n", padding);
int tabStop = 10;
tabStopTest(tabStop, "hello", "friend");
tabStopTest(tabStop, "my name is", "inigo montoya");
tabStopTest(tabStop, "you killed my father", "prepare to die");
}
private static void tabStopTest(int tabStop, String start, String message)
{
StringBuilder builder = new StringBuilder();
builder.append(start);
int padHere = tabStop - builder.length();
if(padHere > 0)
builder.append(" ".repeat(padHere));
else
builder.append(' ');
builder.append(message);
System.out.println(builder);
}
}| StringBuilder b = new StringBuilder(padding); | ||
| while (b.length() < padding) | ||
| b.append(' '); | ||
| this.tagPadding = b.toString(); |
There was a problem hiding this comment.
The entire else branch can be ...
this.tagPadding = " ".repeat(padding);| public void testStdErrLogDebug() | ||
| { | ||
| JettyLoggerConfiguration config = new JettyLoggerConfiguration(); | ||
| Properties props = new Properties(); |
There was a problem hiding this comment.
Would have preferred a new test method, not polluting the existing one.
This new mutated test method is testing too many things now.
|
I created PR #4997 as an alternate approach for this. |
|
Closed in favour of #4997 |
Implemented the TAG_PAD configuration for jetty log appender.
There was a half implementation for thread name padding, but that is not sufficient to align the log output. Tag padding works on the combined thread and logger name.