Need a way to customize fail behavior in menus/param menus #1487

Merged
merged 4 commits into from Oct 2, 2013

Conversation

Projects
None yet
2 participants
@farmdawgnation
Owner

farmdawgnation commented Sep 14, 2013

Currently, if a param menu fails because it can't find the parameter referenced, you get a 404. There's no easy way to customize that behavior for situations where you want something else to happen.

We should implement a LocParam or something that you can tack on to a menu definition that allows you to specify custom fail behavior.

@ghost ghost assigned farmdawgnation Aug 9, 2013

farmdawgnation added some commits Sep 14, 2013

Simplify Loc.doesMatch_?
As I was reading over this code to look at this issue, we were calling
Link.isDefinedAt to determine whether or not to call Link.appy.
Link.apply calls Link.isDefinedAt to determine whether or not to throw
an exception.

This seemed redundant in this particular case, so I've reduced this
method to a simple boolean expression that should have the same effect.
Add and implement the MatchWithoutCurrentValue LocParam.
The MatchWithoutCurrentValue LocParam overrides Lift's default behavior
of considering an Empty currentValue a non-match for a Loc. This default
action is desirable in most circumstances, but occasionally we want to
allow client code to define some custom behavior in the event
currentValue comes up Empty.

Using this new LocParam in combination with IfValue would allow them to
do so.
@fmpwizard

This comment has been minimized.

Show comment Hide comment
@fmpwizard

fmpwizard Sep 14, 2013

Owner

👍

Owner

fmpwizard commented Sep 14, 2013

👍

@fmpwizard

This comment has been minimized.

Show comment Hide comment
@fmpwizard

fmpwizard Oct 2, 2013

Owner

how likely do you think it is that merging this would break any app running 2.6?

if you don't think it is likely, I'd like to merge it and then do 2.6-M1 build this weekend, if you think there is a risk, maybe we can do 2.6-M1 without it, and then merge it right away, so people can start using a somewhat stable 2.6, and then try 2.6-SNAPSHOT to try this out.

I think merging should be ok, but figured to ask just in case :)

Owner

fmpwizard commented Oct 2, 2013

how likely do you think it is that merging this would break any app running 2.6?

if you don't think it is likely, I'd like to merge it and then do 2.6-M1 build this weekend, if you think there is a risk, maybe we can do 2.6-M1 without it, and then merge it right away, so people can start using a somewhat stable 2.6, and then try 2.6-SNAPSHOT to try this out.

I think merging should be ok, but figured to ask just in case :)

@farmdawgnation

This comment has been minimized.

Show comment Hide comment
@farmdawgnation

farmdawgnation Oct 2, 2013

Owner

how likely do you think it is that merging this would break any app running 2.6?

I don't think it's very likely. There are specs that tested Loc at a basic level, and they still pass. This is mostly an addition of new things.

Let's get it in.

Owner

farmdawgnation commented Oct 2, 2013

how likely do you think it is that merging this would break any app running 2.6?

I don't think it's very likely. There are specs that tested Loc at a basic level, and they still pass. This is mostly an addition of new things.

Let's get it in.

fmpwizard added a commit that referenced this pull request Oct 2, 2013

Merge pull request #1487 from lift/msf_issue_1487
Need a way to customize fail behavior in menus/param menus

@fmpwizard fmpwizard merged commit 74a951f into master Oct 2, 2013

@fmpwizard fmpwizard deleted the msf_issue_1487 branch Oct 2, 2013

@fmpwizard

This comment has been minimized.

Show comment Hide comment
@fmpwizard

fmpwizard Oct 2, 2013

Owner

Done :) !

Owner

fmpwizard commented Oct 2, 2013

Done :) !

@farmdawgnation

This comment has been minimized.

Show comment Hide comment
@farmdawgnation

farmdawgnation Oct 2, 2013

Owner

! ! ! ! ! ! ! ! ! ! !

Owner

farmdawgnation commented Oct 2, 2013

! ! ! ! ! ! ! ! ! ! !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment