[GitHub] [tomcat] Holomark edited a comment on pull request #310: Remove extraneous spaces around the colons of the output of the contexts list in the text-based manager

[GitBox <gi...@apache.org>]

Holomark edited a comment on pull request #310:
URL: https://github.com/apache/tomcat/pull/310#issuecomment-652226783


   I understand your comment, but I really believe this principle doesn't apply here. I may have not made my example clear, I am not trying to bend Tomcat to the whims of a plugin.
   
   Here is a sample output from http://localhost:8080/manager/text/list on my machine, with a typical Tomcat installation:
   ```
   OK - Listed applications for virtual host [localhost]
   /:running:0:ROOT
   /webstart:running:0:webstart
   /manager:running:0:manager
   /docs:running:0:docs
   ```
   
   Here is the same output, in the context of a French configured system
   ```
   OK - Applications listées pour l''hôte virtuel (virtual host) [localhost]
   / : running : 0 : ROOT
   /webstart : running : 0 : webstart
   /manager : running : 0 : manager
   /docs : running : 0 : docs
   ```
   
   As you can expect, an automated script splits the line on the colons. I believe this is a fair assumption, as far as I understand the text-based manager _is_ meant to be script-friendly.
   This is not about translation, this is breaking the contract.
   
   Actually I'd say the initial mistake was to put this line with the localized strings.
   I was tempted to propose the following correction:
   Those lines [from the ManagerServlet](https://github.com/Holomark/tomcat/blob/21d2000c99134b7ec54424ea210cf9b2fd6aae38/java/org/apache/catalina/manager/ManagerServlet.java#L1079)
   ```java
                   if (context.getState().isAvailable()) {
                       writer.println(smClient.getString("managerServlet.listitem",
                               displayPath,
                               "running",
                               "" + context.getManager().findSessions().length,
                               context.getDocBase()));
                   } else {
                       writer.println(smClient.getString("managerServlet.listitem",
                               displayPath,
                               "stopped",
                               "0",
                               context.getDocBase()));
                   }
   ```
   ... would be tweaked to:
   ```java
                   if (context.getState().isAvailable()) {
                       writer.println(String.join(":",
                               displayPath,
                               "running",
                               "" + context.getManager().findSessions().length,
                               context.getDocBase()));
                   } else {
                       writer.println(String.join(":",
                               displayPath,
                               "stopped",
                               "0",
                               context.getDocBase()));
   ```
   ... and the `managerServlet.listitem` line in LocalStrings_xx.properties removed (It is not used elsewhere).
   
   I stepped back and proposed only a correction that seemed more innocuous to `LocalStrings_fr.properties`, as I have never contributed to this project, and barely in any other open source project.
   
   Am I wrong ? I may be missing something.
   
   Thank you for your time.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org