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

inspec demo #989

Merged
merged 5 commits into from
Sep 1, 2016
Merged

inspec demo #989

merged 5 commits into from
Sep 1, 2016

Conversation

vjeffrey
Copy link

@vjeffrey vjeffrey commented Aug 26, 2016

ok, so obviously this is not ready ready, but it's a start, so i'm putting up a PR for it...i think this works for the whole "viability gate" thing. There is much more to do, but i think this is a good spot to commit something. Please list out ANYTHING you can think of that you see needing improvement/want implemented, so i can keep a list as I truck along.
fixes most of #956


evalInput(value) {
if (value.match(/^inspec exec\s*examples\/profile\/controls\/example.rb\s*/)) {
this.getResponse(value, "inspec_exec");
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes sense for the viability demo, but I think we might want to bundle the response data (or a segment of it) in a single blob that we send to the client to avoid a lot of round tripping.

@stevendanna
Copy link
Contributor

We probably want to either strip the color control characters from our recording or figure out how to translate those into the right colors in the browser:

screen shot 2016-08-26 at 5 28 21 pm

@stevendanna
Copy link
Contributor

It is hard to get a screenshot of, but the cursor appears on the line below the $ rather than on the same line like it would in a terminal. Also, when you hit enter on your command, the command you entered moves down a line while the behavior in the terminal would be for the output to appear and then a new prompt to appear with your cursor on it.

@stevendanna
Copy link
Contributor

stevendanna commented Aug 26, 2016

Like the pattern matching on the errors, very cool, but it seems to have gone wrong a little bit:

inspec exec /does/not/exist

 Could not fetch inspec profile in "examples/profile/example.rb".

@stevendanna
Copy link
Contributor

The instructions don't seem to reposition if I make my browser window smaller.

@stevendanna
Copy link
Contributor

@vjeffrey Listed some issues I found, but overall this was very easy to get running and worked immediately, very cool start.

if (value.match(/^inspec exec\s*examples\/profile\/controls\/example.rb\s*/)) {
this.getResponse(value, "inspec_exec");
}
else if (value.match(/^inspec exec\s*.*/)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to find a way to parameterize responses so that we could capture the path here and inject it into the response we print.

@vjeffrey
Copy link
Author

i'm working on a refactor commit that will pull the terminal view into its own component to make real shell emulation easier-- up arrow thru previous commands, prompt -command-repeat cycle -- and it will also reduce the amount of get requests.

@chris-rock
Copy link
Contributor

@Vj you probably want to use https://github.com/drudru/ansi_up, this helps with the colors @stevendanna mentioned

@chris-rock
Copy link
Contributor

Maybe https://github.com/sourcelair/xterm.js also helps with the rendering of a terminal

}

evalInput(value) {
if (value.match(/^inspec exec\s*examples\/profile\s*/)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

During generation of the commands, we probably want to have json file that tracks the commands. We could use https://github.com/chef/inspec/blob/master/tasks/command_simulator.rb#L4 as an external json. This enables us to use it as a reference in this app and as a list of commands for the rake task

@chris-rock
Copy link
Contributor

This is a really great start @vjeffrey ! I see two points that we can improve:

  • create a json for all the commands, so that we can generate those (enables us to change only one file and use it for generation and web service )
  • improve rendering of the terminal e.g. reuse a term library available

@chris-rock
Copy link
Contributor

Maybe we should move this to www/tutorial directory. this allows us to use the root www dir for the general app.

@vjeffrey
Copy link
Author

just pushed up a commit i had started on friday to cleanup and refactor a bit/reduce get requests. also moved to tutorial folder. now i'm starting to work on getting xterm in, then once that's done i'll focus on command generating (json) and then the ui bits of making it look nice and ansi_up and stuffs

@vjeffrey
Copy link
Author

vjeffrey commented Aug 31, 2016

@chris-rock @arlimus updated with the refactor/addressing comments. i still need to figure out the print a block of text and don't make it look crazy thing....working on it...
something else seems to have gone haywire as well - i deleted my node_modules and re-did npm install and all that and now it's saying Terminal isn't defined -- i'm looking into it

@vjeffrey
Copy link
Author

vjeffrey commented Sep 1, 2016

holybergeeeeeez it took me forever to figure out everything was broken because xterm.js had some updates in the couple days. thus the pinning to 1.1.2 commit.

@vjeffrey
Copy link
Author

vjeffrey commented Sep 1, 2016

@chris-rock @arlimus try it out I fixed things with @stevendanna's help :) :) :)

@@ -0,0 +1,20 @@
Commands:
Copy link
Contributor

Choose a reason for hiding this comment

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

A note on this: The problem we found is that the xterm js library expect windows style line endings \r\n. The rake task now generates files with the correct endings, but we may need to be vigilant to ensure that git doesn't mangle them.

@chris-rock
Copy link
Contributor

Very nice first iteration @vjeffrey

@vjeffrey
Copy link
Author

vjeffrey commented Sep 1, 2016

just pushed up another small commit -- realized i wasn't handling next and prev very well

@chris-rock chris-rock merged commit 8616abf into master Sep 1, 2016
@chris-rock chris-rock deleted the vj/inspec-demo branch September 1, 2016 22:52
@chris-rock chris-rock modified the milestone: 0.33.0 Sep 2, 2016
@chris-rock chris-rock added the Aspect: Docs Write the Fine Manual label Sep 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aspect: Docs Write the Fine Manual
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants