Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some cherries you might like to pick #3

Closed
wants to merge 4 commits into from

Conversation

nickl-
Copy link
Contributor

@nickl- nickl- commented Jun 8, 2018

Not necessary to merge, submitted to make it easier to share my discoveries. Cherry pick what you'd like to keep.

Much much better experience this time around =) the shell scripts are a welcome addition.

I worked out what CommandLineReader does to get the prompt to come back, it sends ; to the interpreter on blank lines... magic!

Also found the guy that handles the completion for the JConsole frontend, it also exposes the classpath and much simpler to implement.

Took a stab at diving deeper for member completion... this may need another visit to the drawing board but as proof of concept it has value. What you think?

Good stuff!!

So bshell starts up on top of the screen with no clutter.
It would appear that org.jline.utils.Display.clear() may also attempt to accomplish this but I haven't seen it do so yet.
I discovered a little gem in bsh.util.NameCompletionTable it's the guy that does the JConsole completion.
Gets bootstrapped with the interpreter namespace and can also work the classpath. Do try with JDK8 with rt.jar loaded for pretty neat insights, Java 9 currently can't get that detail.
The whole MappingFeedback listener is a tad odd, without it the ClassPath echo's to stderr what it's doing.
The rest is an attempt to dive deeper obtaining members for completion, it uses bsh.Name (nameResolver)'s magic which can be a bit like a magicians top hat, you never know what you're going to get. Good proof of concept me thinks.
@stefanofornari
Copy link
Owner

merging nick's changes. some comments will follow by email

Copy link
Owner

@stefanofornari stefanofornari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see the spice on my terminal, what was the intended behaviour?

@stefanofornari
Copy link
Owner

i am not sure I have done it right .... reopening (it is my first pull request :/)

Copy link
Owner

@stefanofornari stefanofornari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what -s is for? it does not work on ubuntu linux; I removed it for now, which along should work (I guess...)

Copy link
Owner

@stefanofornari stefanofornari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty cool. I will look into the space problem. The result with jline is cool. I need also to work on some specs for it (similar to unit test...).
one important note: this code does not work with bsh-2.0b6, it needs the development snapshot. When do you plane to cut a release of beanshell 2?

Copy link
Owner

@stefanofornari stefanofornari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually not desirable: it fails when you want to write multi-line code (e.g. print(

"hello"

);

@stefanofornari
Copy link
Owner

stefanofornari commented Jun 11, 2018 via email

@nickl-
Copy link
Contributor Author

nickl- commented Jun 15, 2018

  1. on the prompt, I am ok with the change but I could not see the effects in my console. what was the intended behaviour?

Lets see if pictures will speak 1000 words...

This is what I see in iTerm on macOS... but gnome terminal should support all the niceties too, not?
Testing multiline... if you don't leave any blank lines it appears to work fine. I am sure we can add a bit more intelligence to the blank line rule.

Code completion tab after typing I...n...t...e
screen shot 2018-06-15 at 09 35

Tab after the .
screen shot 2018-06-15 at 09 36

Type M..A..X..tab and enter.
screen shot 2018-06-15 at 09 37

  1. on completion, I am still working on it, but I would merge the code you provided

It was not meant as a merge PR just for sharing and maybe you can pick a few lines that you like. I think I decided to abort the direction I was heading in but instead of just discarding the code I thought it might be useful to show you. The solution is not ideal, I was using bsh.Name which seems like a magicians top hat... you never know what you're going to get out.

Name is basically the go to guy in bsh for when we get something from the parser but we don't know what it is, all we do know is that it is a name. Name goes in one end, it does try and be clever about it and is eager to find something rather than fail or give up. Out the other end comes the name resolved to whatever stuck. Which does seem similar in requirements to what we need for completion, doesn't it?

bsh.util.NameCompletionTable on the other hand is a good idea!!... still needs work but it makes good sense. I have implemented the missing rt.jar classpath for Java 9/10 by allowing us to search through the java.base module for available classes. The screenshots were run from Java 10. I think this is a good starting point for completion happiness.

does not work with bsh-2.0b6

Yes most definitely not that release... have you not looked at the new stuff yet? I don't even recognise the source code on master anymore. Use the new beanshell it actually works.

When do you plane to cut a release of beanshell 2?

Hmmm was thinking beanshell3 maybe, to avoid any confusion... but then I thought what about beanshell8?... as in java 8 minimum required and we take it from there... dunno. sigh Didn't choose to be the one making all the decisions for beanshell it just kind of happened. Really wish there was more activity from the community, where are all the maintainers??? Some help with making decisions, at least.

It's not ready for release yet. There are still a few things rubbing me the wrong way which I would like to finalise before merging back to master. Maybe I should write up an "outstanding todo list/roadmap to release" or something. My aim is to stop adding new features, the focus is on test coverage, code sanitising, testing and bug fixes. It still feels like a can a worms, lift one corner and bugs scatter in every direction. =)

Pretty cool. I will look into the space problem. The result with jline is cool. I need also to work on some specs for it (similar to unit test...).

It is encouraging and super fabulously kewl to know/see that there is something else exciting happening with beanshell while I continue with the boring grind. I should keep my focus on the release path though... would be a shame if I get distracted before I finish. We are so close!

Looking forward to seeing your progress.

@stefanofornari
Copy link
Owner

thanks nick for the explanation. I want to fix the additional space issue first, then I will look into the above. this will buy us also some time for the release. I am trying the new stuff, and I look forward to switch completely, but being not available on maven central, I would like the standard distribution to work with released packages.

I've got the coloring, thanks. But I need help for the prompt. I am opening a thread on the beanshell list.

BTW, my vote is for bsh8 (or 9 or 10 :) ).

@stefanofornari
Copy link
Owner

I am not mistaking, I should have incorporated all the agreed suggestions. I am closing this one, feel free to reopen if I missed anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants