Add a configuration that can enable/disable block comment processing #488

Activity

1. jvalkeal commented on Jul 29, 2022

   

   jvalkeal

   on Jul 29, 2022

   Contributor

   More actions

   1.2.0.RELEASE is 6 years old release so can you try 2.1.0.

   To you question I have a hard time to understand what you mean. Is this about command file? Can you show a real example what happens with a shell.
2. paul8263 commented on Jul 29, 2022

   

   paul8263

   on Jul 29, 2022

   Author

   More actions

   Hi @jvalkeal ,

   Thank you very much for your reply.

   It is about command line param. As shown in the example below:

   public class MainShell {
       public static void main(String[] args) throws Exception {
           Bootstrap bootstrap = new Bootstrap();
           JLineShellComponent shell = bootstrap.getJLineShellComponent();
           shell.executeCommand("command --str */*/*");
       }
   }

   @Component
   public class MyCommand implements CommandMarker {
       @CliCommand(value = "command")
       public String execCommand(
               @CliOption(key = {"str"}) String str
       ) throws Exception {
           return str;
       }
   }

   The problem is, when we pass */*/* as the parameter 'str' but actually we get */**。
   It seems this feature cannot be turned off . The relevant code block is:

   AbstractShell::executeCommand:

   // We support simple block comments; ie a single pair per line
   if (!inBlockComment && line.contains("/*") && line.contains("*/")) {
       blockCommentBegin();
       String lhs = line.substring(0, line.lastIndexOf("/*"));
       if (line.contains("*/")) {
           line = lhs + line.substring(line.lastIndexOf("*/") + 2);
           blockCommentFinish();
       } else {
           line = lhs;
       }
   }
   if (inBlockComment) {
       if (!line.contains("*/")) {
           return new CommandResult(true);
       }
       blockCommentFinish();
       line = line.substring(line.lastIndexOf("*/") + 2);
   }
   // We also support inline comments (but only at start of line, otherwise valid
   // command options like http://www.helloworld.com will fail as per ROO-517)
   if (!inBlockComment && (line.trim().startsWith("//") || line.trim().startsWith("#"))) { // # support in ROO-1116
       line = "";
   }

   Also I got two more questions and need some help:

   1. Can spring shell 2.1.0 temporarily turn off this feature by centain configuration?
   2. It seems that 2.1.0 version works with spring boot, but we prefer a light-weight implementation. I prefer to upgrade but have to take it into consideration.
3. jvalkeal commented on Jul 29, 2022

   

   jvalkeal

   on Jul 29, 2022

   Contributor

   More actions

   This is from hoodie right? Unfortunately you are on an unsupported stack here as this is such an old code. Part of that code in shell 1.x code originates from spring roo(which means we're talking 10+ years). Spring Boot then became later which these days is the bootstrapping framework.

   If you need to fix something on spring-shell 1.x I think only option is to fork it and make needed quick fixes to give more time for possible rework/overhaul on top of more modern spring-shell.

   Since early days of spring shell 1.x it has been rewritten two times so very little is in place these days. What comes for your question 1, it's not valid as offending code is not there anymore.

   About your question 2; Spring shell 1.x used spring-framework as spring-boot didn't exist at that time, thus the Bootstrap you have. You'd be surprised how good solution spring-shell is with spring-boot as there's been a years of maintenance in a framework itself.

   Also if you choose to migrate, you have a good path forward to keep dependencies up-to-date, get a change to be on a supported versions from both spring and jdk.
4. jvalkeal commented on Jul 29, 2022

   

   jvalkeal

   on Jul 29, 2022

   Contributor

   More actions

   I think it's better to move this into discussion.
5. spring-projects

   locked and limited conversation to collaborators on Jul 29, 2022
6. jvalkeal

   converted this issue into a discussion #492 on Jul 29, 2022