Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Modify cookie[]= method to support additional options #66

Merged
merged 1 commit into from Feb 20, 2013

Conversation

Projects
None yet
4 participants
Contributor

kylewlacy commented Sep 23, 2012

From what I understand, when using Sinatra::Cookie, if a cookie needed to have extra options (such as :expire, for example), then the global cookie options needed to change. I've modified the []= method in the Jar class to support such extra options. For example, the following syntax would set an expiration on the ':foo' cookie:

cookie[:foo, :expires => 30*24*60*60] = 'bar'
# The 'foo' cookie will now expire in 30 days

The usefulness of this seems obvious, and I'm surprised that this hasn't been done before. I've also written an accompanying spec for this behavior, while all previous specs passed without modification.

@kylewlacy kylewlacy modify cookie[]= to support extra cookie options
Using the syntax `cookie[:key, :options => 'hash']='value'`` will now
merge the provided hash with the existing @options hash (which allows
for changing cookie options on a per-cookie basis).
44caa86

i need that.thx

@TrevorBramble TrevorBramble added a commit that referenced this pull request Feb 20, 2013

@TrevorBramble TrevorBramble Merge pull request #66 from kylewlacy/master
Modify cookie[]= method to support additional options
1ad87b6

@TrevorBramble TrevorBramble merged commit 1ad87b6 into sinatra:master Feb 20, 2013

Contributor

TrevorBramble commented Feb 20, 2013

I can't see a reason why this shouldn't have been supported either. Thanks for the addition, @kylewlacy!

@TrevorBramble TrevorBramble added a commit that referenced this pull request Feb 20, 2013

@TrevorBramble TrevorBramble Reverting due to Ruby 1.8.7 incompatibility
Revert "Merge pull request #66 from kylewlacy/master"

This reverts commit 1ad87b6, reversing
changes made to 2141511.
1107072
Contributor

TrevorBramble commented Feb 20, 2013

Reverted due to busting 1.8.7. Not a fan of the proposed syntax anyway. Perhaps an optional block? or value optionally being a hash (if so, with required :value key)?

Owner

rkh commented Feb 20, 2013

Or a store method that is used by []=?

Contributor

kylewlacy commented Feb 24, 2013

I changed the syntax around to accept a hash (as suggested). For instance, the example above would now look like this:

cookie[:foo] = {:value => 'bar', :expires => 30*24*60*60}
# The 'foo' cookie will now expire in 30 days

The new commit is at kylewlacy/sinatra-contrib@9c7240d. Also, can you reopen the issue? I can't seem to add the commit to the issue otherwise.

Owner

rkh commented Feb 24, 2013

For me that's counter intuitive, tbh.

Contributor

TrevorBramble commented Feb 25, 2013

I'm a silly. Should have suggested this.

def []=(key, value, options = {})
  @response.set_cookie key.to_s, @options.merge(options).merge(:value => value)
end

cookie['foo'] = 'bar', :expires => 30*24*60*60

@kylewlacy Sorry, there doesn't seem to be a way for me to re-open the pull. If you can't either, I guess there has to be a new one. =^/

@zzak zzak pushed a commit to zzak/sinatra-contrib that referenced this pull request Jul 22, 2016

@TrevorBramble TrevorBramble Merge pull request #66 from kylewlacy/master
Modify cookie[]= method to support additional options
b5293a7

@zzak zzak pushed a commit to zzak/sinatra-contrib that referenced this pull request Jul 22, 2016

@TrevorBramble TrevorBramble Reverting due to Ruby 1.8.7 incompatibility
Revert "Merge pull request #66 from kylewlacy/master"

This reverts commit 1ad87b6, reversing
changes made to 2141511.
a03684c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment