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

Add no-save feature #4

Merged
merged 4 commits into from
May 17, 2017
Merged

Add no-save feature #4

merged 4 commits into from
May 17, 2017

Conversation

morishin
Copy link
Contributor

Closes #3

I implemented autoremove feature.

  • Toybox creates a playground file named <name>.autoremove.playground with --rm option.
  • The files which has .autoremove suffix are not shown by toybox list command.
  • Toybox removes the files with .autoremove suffix every time toybox command has been called.

Please review it @giginet

public var rootURL: URL {
return Workspace.rootURL
}

public init() {
executeAutoremove()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in the initializer because I want to remove files every command execution. Do you have any better idea?

@giginet giginet mentioned this pull request May 16, 2017
@morishin morishin force-pushed the autoremove-feature branch from 3394222 to c491bb4 Compare May 16, 2017 14:00
@morishin morishin force-pushed the autoremove-feature branch from c491bb4 to 01e149b Compare May 17, 2017 07:40
@morishin morishin force-pushed the autoremove-feature branch from 01e149b to 68b1522 Compare May 17, 2017 07:43
Copy link
Owner

@giginet giginet left a comment

Choose a reason for hiding this comment

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

LGTM

Could you add an example to README?
I prefer to add shorthand. Do you have any idea?

@@ -38,6 +40,7 @@ struct CreateOptions: OptionsProtocol {
<*> m <| Switch(flag: "f", key: "force", usage: "Whether to overwrite existing playground")
<*> m <| Switch(key: "no-open", usage: "Whether to open new playground")
<*> m <| Switch(key: "input", usage: "Whether to enable standard input")
<*> m <| Switch(key: "no-save", usage: "Remove playground file automatically")
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer to also add shorthand. (But I have no idea....)

Copy link
Contributor Author

@morishin morishin May 17, 2017

Choose a reason for hiding this comment

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

I votes -n (though I know it may cause confusion with --no-save --no-open...)

Copy link
Owner

Choose a reason for hiding this comment

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

We also have no-open option...

@@ -63,6 +71,23 @@ class HandlerTests: XCTestCase {
XCTAssertTrue(manager.fileExists(atPath: playgroundURL(for: "hello").path))
}

func testCreateWithTemporaryOption() {
_ = handler.create("hello", for: .iOS, temporary: true)
Copy link
Owner

@giginet giginet May 17, 2017

Choose a reason for hiding this comment

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

We should use @discardableResult. (I'm goint to add after this.)

Copy link
Owner

@giginet giginet left a comment

Choose a reason for hiding this comment

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

LGTM

@giginet giginet changed the title Add autoremove feature Add no-save feature May 17, 2017
@giginet giginet merged commit 5734d02 into giginet:master May 17, 2017
@morishin
Copy link
Contributor Author

morishin commented May 17, 2017

Thanks 🎉

@morishin morishin deleted the autoremove-feature branch May 17, 2017 13:55
@giginet
Copy link
Owner

giginet commented May 17, 2017

Thank you @morishin ❤️

Upgrade Toybox to 0.2.0 with Homebrew to use new feature.

$ brew upgrade giginet/toybox/toybox

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.

auto remove option
2 participants