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

Persistent command history on Loop #75

Open
javierbarre opened this issue Aug 26, 2018 · 5 comments
Open

Persistent command history on Loop #75

javierbarre opened this issue Aug 26, 2018 · 5 comments

Comments

@javierbarre
Copy link

Is it possible to make the command history persistent? Or save the commands entries to a local file?

In case there is not, can you provide Any example on how to access to the command history (_entries), and send a command to the prompt for execution?

@reubeno
Copy link
Owner

reubeno commented Aug 26, 2018

It’s theoretically possible today using the exposed API, but would require too much work for the app. The right thing to do is for us to either add history persistence as a configurable feature or expose the command history interface as replaceable by the calling app.

@javierbarre if I provided some guidance / pointers, are these changes something you might be interested in making in nclap and contributing back via PR?

@javierbarre
Copy link
Author

@reubeno with some guidance, yes I am interested. Thanks

@reubeno
Copy link
Owner

reubeno commented Aug 28, 2018

I took another look through the code. For Loop clients that provide a LoopInputOutputParameters object into the Loop constructor, there might be a simple option. You could add a new property to that parameters class -- allowing it to take in an IConsoleHistory object (optionally). It might be a little confusing, as if the user is also specifying an IConsoleLineInput object, then the history object would be ignored, but if they aren't, then the CreateClient helper in Loop.cs could pass the provided history object into the constructor for the internal-only ConsoleLineInput class.

What do you think, @javierbarre ? That could be a starting point. The application could then implement IConsoleHistory to provide persistence. Down the road, if we had a good reusable implementation of a persisted console history, we could consider folding that into nclap too.

@javierbarre
Copy link
Author

Great. I implemented persistence through the application as your suggested by implementing a new IConsoleHistory.

Modifications:
In LoopInputOutputParameters.cs I added:

        /// <summary>
        /// Console History
        /// </summary>
        public IConsoleHistory ConsoleHistory { get; set; }

And in CreateClient of Loop.cs:

        /// <summary>
        /// Utility for constructing loop clients.
        /// </summary>
        /// <param name="parameters">I/O parameters for loop.</param>
        /// <returns>A constructed loop client.</returns>
        public static ILoopClient CreateClient(LoopInputOutputParameters parameters)
        {
            var consoleInput = parameters.ConsoleInput ?? BasicConsole.Default;
            var consoleOutput = parameters.ConsoleOutput ?? BasicConsole.Default;
            var keyBindingSet = parameters.KeyBindingSet ?? ConsoleKeyBindingSet.Default;
            var consolehistory = parameters.ConsoleHistory ?? new ConsoleHistory(); // NEW LINE
            var lineInput = parameters.LineInput ?? new ConsoleLineInput(
                consoleOutput,
                new ConsoleInputBuffer(),
                consolehistory);   // MODIFIED LINE

            lineInput.Prompt = parameters.Prompt ?? Strings.DefaultPrompt;

            var consoleReader = new ConsoleReader(lineInput, consoleInput, consoleOutput, keyBindingSet);

            var consoleClient = new ConsoleLoopClient(consoleReader);

            consoleClient.Reader.CommentCharacter = parameters.EndOfLineCommentCharacter;

            return consoleClient;
        }

I needed to disable the TreatWarningsAsErrors as I am getting two warnings:

Repl\LoopInputOutputParameters.cs(47,49): warning RS0016: Symbol 'ConsoleHistory.get' is not part of the declared API.
Repl\LoopInputOutputParameters.cs(47,54): warning RS0016: Symbol 'ConsoleHistory.set' is not part of the declared API.

How can I get rid off these warnings?

@reubeno
Copy link
Owner

reubeno commented Sep 15, 2018

This is part of the mechanism we use to make sure we don't accidentally change the exposed interface of nclap without it being a conscious decision. If you navigate to the symbol in question, you should see a colored squiggly line beneath it in Visual Studio. There should also be a lightbulb icon that you can click on to auto-fix the issue; "fixing it" will add the API to the accepted allow-list of public API surface.

If you have continued difficulty with this, please let us know.

P.S. Sorry for the delay!

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

No branches or pull requests

2 participants