GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — implemented CommandLine.UnexpectedOption by adamralph · Pull Request #896 · scriptcs/scriptcs

Conversation

@adamralph

@adamralph adamralph commented Jan 7, 2015

Copy link
Copy Markdown
Contributor

fixes #869

needs to be merged before we can release 0.13 - see #869 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@adamralph
red: added CommandLine.UnexpectedOption
f614cc8
@adamralph adamralph added the child PR label Jan 7, 2015
@adamralph

adamralph commented Jan 7, 2015

Copy link
Copy Markdown
Contributor Author

There are plenty more opportunities to refactor and simplify the arguments implementation but I decide to draw a line under what I've done so far and send this PR, since we need to close #869 before the 0.13 release, and I need to get back onto #626 which also needs to be fixed for 0.13 to go live.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph adamralph mentioned this pull request Jan 8, 2015
CLI args/config file refactoring #897
Closed
@adamralph

adamralph commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

I did a test rebase of my work on closing #626 onto this branch and there were some tricky conflicts so I decide to resolve them and leave my work for #626 based on this branch.

This means the sooner we can get this PR merged the better, since it's now the blocker for all work required to push out 0.13.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

BTW, the easiest way to review this PR may be one commit at a time, since each is well isolated with a specific purpose.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@khellang

khellang commented Jan 8, 2015

Copy link
Copy Markdown
Member

Removing nesting doesn't diff very well in general 😉
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

Some local diff apps do it quite well if you toggle off 'compare whitespace'. I sent a feature request to GitHub for that a while back but they've not responded so far.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@khellang

khellang commented Jan 8, 2015

Copy link
Copy Markdown
Member

That's actually very true! Tried https://github.com/scriptcs/scriptcs/pull/896/files?w=1, works like a charm. You knew about ?w=1, right?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Jan 8, 2015

Copy link
Copy Markdown
Contributor Author

I'd forgotten about it, but thanks for reminding me 😉. I guess it would ultra trivial to add a UI control for it then...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@khellang

khellang commented Jan 8, 2015

Copy link
Copy Markdown
Member

LOL. Yeah, it's really weird that it they haven't added it yet. The query param has been there for years.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
adamralph added 2 commits January 10, 2015 16:07
@adamralph
green: implemented CommandLine.UnexpectedOption
0eaaae9
@adamralph
refactor: ScriptDirectory/ScriptCsExe naming, overloads and formatting
7a31f10
@adamralph

adamralph commented Jan 10, 2015

Copy link
Copy Markdown
Contributor Author

@scriptcs/core I decided to remove the arguments/config file refactoring commits from this PR because they were bloating it and I think that whole space actually needs a re-write instead (see #897).

This PR is much simpler now and only contains one refactoring commit for the acceptance tests. Would anyone have time to look at it over the weekend?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph

adamralph commented Jan 16, 2015

Copy link
Copy Markdown
Contributor Author

Can anyone look at this? We need to move forward with the nightly because the situation with the folder migration is already starting to confuse people, e.g. scriptcs/scriptcs-samples#20.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@khellang khellang added this to the v0.13 milestone Jan 16, 2015
khellang added a commit that referenced this pull request Jan 16, 2015
@khellang
Merge pull request #896 from adamralph/895
c97d79f 
implemented CommandLine.UnexpectedOption
@khellang
khellang merged commit c97d79f into scriptcs:dev Jan 16, 2015
@adamralph

adamralph commented Jan 16, 2015

Copy link
Copy Markdown
Contributor Author

Awesomeness dude. PR for #626 coming soon...
On 16 Jan 2015 09:26, "Kristian Hellang" [email protected] wrote:

Merged #896 #896.

—
Reply to this email directly or view it on GitHub
#896 (comment).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@adamralph
adamralph deleted the 895 branch January 16, 2015 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

child PR

Projects

None yet

Milestone

v0.13

Development

Successfully merging this pull request may close these issues.

Default to REPL when other options are passed

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@adamralph @khellang
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.