Skip to content

Conversation

@adamralph
Copy link
Contributor

fixes #869

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

@adamralph
Copy link
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.

@adamralph
Copy link
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.

@adamralph
Copy link
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.

@khellang
Copy link
Member

khellang commented Jan 8, 2015

Removing nesting doesn't diff very well in general 😉

@adamralph
Copy link
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.

@khellang
Copy link
Member

khellang commented Jan 8, 2015

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?

@adamralph
Copy link
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...

@khellang
Copy link
Member

khellang commented Jan 8, 2015

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

@adamralph
Copy link
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?

@adamralph
Copy link
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.

@khellang khellang added this to the v0.13 milestone Jan 16, 2015
khellang added a commit that referenced this pull request Jan 16, 2015
implemented CommandLine.UnexpectedOption
@khellang khellang merged commit c97d79f into scriptcs:dev Jan 16, 2015
@adamralph
Copy link
Contributor Author

Awesomeness dude. PR for #626 coming soon...
On 16 Jan 2015 09:26, "Kristian Hellang" notifications@github.com wrote:

Merged #896 #896.


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

@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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default to REPL when other options are passed

2 participants