jline / jline3

JLine is a Java library for handling console input.
Other
1.49k stars 218 forks source link

Parser.parse()'s "cursor" argument is not the cursor position for ACCEPT_LINE #84

Closed scgray closed 7 years ago

scgray commented 7 years ago

The definition of the method is:

ParsedLine parse(String line, int cursor, ParseContext context) throws SyntaxError;

default ParsedLine parse(String line, int cursor) throws SyntaxError {
    return parse(line, cursor, ParseContext.UNSPECIFIED);
}

but when called for ACCEPT_LINE the "cursor" argument is actually always line.length():

try {
    parsedLine = parser.parse(str, str.length(), ParseContext.ACCEPT_LINE);
} catch (EOFError e) {

(this is from LineReaderImpl.acceptLine().

For my purposes I really need to know the context in which the user hit accept line (presumably ENTER) -- that is, I need the "cursor" variable to mean what it's name indicates it should mean. For example, in my application a semicolon at the end of a line usually means "run it", but in the case of:

prompt> select * _from x;

if _ is the cursor position and the user hits enter, it would make logical sense for it to do:

prompt> select *
prompt> _from;
gnodet commented 7 years ago

I don't mind passing the cursor position of course, though one problem will be when an history event has been expanded, in which case the cursor position will be lost. Another option would be to bind the <enter> key to a custom widget which in turn would either insert a newline or call the accept widget.

scgray commented 7 years ago

Thanks Guillaume - I don't see a lot of documentation or examples of widgets right now (as an aside, I am planning on forking and writing some javadocs as I learn things). I'm not quite sure I see how a widget would have access to enough state information to be able to decipher the current cursor position and buffer contents -- at least not without assuming direct access to LineReaderImpl.

As for the history events, my application doesn't use them so that wouldn't be a problem for me.

As I write this, though, I'm having my doubts as to my idea of the behavior for this issue...I think users might find the behavior strange when sometimes, means newline and in other cases means "run". Is it possible to bind CTRL-ENTER to ACCEPT_LINE and have just ENTER be newline?

gnodet commented 7 years ago

Have a look at the 2 tests in the file below, which demonstrates the use of a custom widget and your second suggestion. https://github.com/jline/jline3/blob/master/reader/src/test/java/org/jline/reader/impl/WidgetTest.java

gnodet commented 7 years ago

Fwiw, there's no simple way to find out a Ctrl+<enter>, that's because the <enter> key is already in the first 32 ascii codes, so it's already mapped to Ctrl+J.

scgray commented 7 years ago

Looking over your examples (thanks for that, I'm trying it now), I notice what appears to be a breaking of abstraction. You have interfaces for LineReader and Buffer, but I notice that some of what you are showing requires using methods on the *Impl classes. For example, LineReader.getBuffer() returns the actual BufferImpl in the definition of the method and reader.getKeyMap() is only defined in the Impl.

Maybe either those methods should be promoted to the interface, or maybe you shouldn't have the interface at all and just make them direct classes?

Also, one of the down sides of the widget approach is that it depends upon the keymap name and with vi your keymap name switches depending on mode (insert vs. command)

gnodet commented 7 years ago

Yes, I'm aware of the BufferImpl problem. It will have to be fixed as part of #70 I think. I'll fix the getKeyMap() one asap. I've raised #86 for the API enhancements.

scgray commented 7 years ago

Ok, I got the widget to work, but I discovered that the enter key is actually CTRL-M. The other issue that I can now is that I need to review each key that can cause ACCEPT_LINE to be called for each keymap :-(. I think I would like to request if you could attempt to make "cursor" accurate when possible, that would make what I'm trying to do less error prone, particularly since I don't use history expansion.