spring-projects / spring-shell

Spring based shell
http://projects.spring.io/spring-shell/
Apache License 2.0
723 stars 395 forks source link

Abandon ExtendedDefaultParser and reuse the default implementation of JLine #517

Open guang384 opened 2 years ago

guang384 commented 2 years ago

In the startup log, you will always see the following WARNING : The Parser of class org.springframework.shell.jline.ExtendedDefaultParser does not support the CompletingParsedLine interface. Completion with escaped or quoted words won't work correctly.

That's because JLine has redefined and implemented the CompletingParsedLine interface since version 3.7.0 Improve support for completion with quotes, #245

In the current implementation, the classes related to ExtendedDefaultParser seem to have directly copied and pasted the previous version of JLine code. Since JLine has made new changes to this interface definition, it is better to give up and reuse the default implementation of jline.

If possible, I would be happy to submit a PR on this issue.

jvalkeal commented 2 years ago

This would be a good thing to do as having less tweaks over jline is better. Its hook to current code show below (not so obvious way):

https://github.com/spring-projects/spring-shell/blob/cf7c409d1c391e5f19cd3274801e4447b86e7fc9/spring-shell-autoconfigure/src/main/java/org/springframework/shell/boot/CompleterAutoConfiguration.java#L53

guang384 commented 2 years ago

I read the source code of the relevant part of JLine and found that a wrap method is provided in org.jline.reader.impl.LineReaderImpl, which directly wraps org.jline.reader.ParsedLine into org.jline.reader.CompletingParsedLine. This behavior would have occurred in the process of docomplete.

In addition, I also found that the DefaultParser has many adjustments in the recent version of JLine, so my suggestion is to directly use the warp method to convert the ExtendedArgumentList into CompletingParsedLine in order to prevent the WARNING log from disturbing the user during startup.