axr / core

The AXR core library
axrproject.org
GNU General Public License v3.0
93 stars 15 forks source link

Rename takeFlag() or revise its behaviour to match its current name #256

Open Mouvedia opened 10 years ago

Mouvedia commented 10 years ago

Finally we have the takeFlag() function which deactivates an existing flag to activate it on the current element(s).

Check the example from the spec.

Currently it activates the flag no matter what. The flag needs to be activated on at least one of the elements matched by the selector.

veosotano commented 10 years ago

The flag needs to be activated on at least one of the elements matched by the selector.

Why?

Usecase: elements of a menu. You click on one and nothing has been selected yet (maybe the page you're on right now doesn't appear in that menu). You want to activate that menu item, doesn't matter that nothing has it yet.

This should be discussed in a spec meeting if necessary.

Mouvedia commented 10 years ago

@veosotano It seems clear from the name that you have to get the flag from something else. Well that's what valerij and I understood at least. I don't understand how it could activate a flag if it didn't actually take it first.

BTW to clarify any misunderstanding when I say "matched by the selector" I mean the inner selector (of the function).

veosotano commented 10 years ago

If you follow the metaphor into the real world, yes, there needs to be something somewhere to be able to take it.

But we are dealing with a language for designing an interface, and in the interface world, creating the flag with takeFlag(), even when no element has it yet, is perfectly reasonable. It would be confusing and very much less useful if it wasn't the case.

Remember also that takeFlag() has always been just an alias for

unflag(foo of !@this), flag(foo);

I'd be interested in how you would use takeFlag() on a menu without active (w flag applied) elements, if it was behaving like you propose, since you could never activate any element, because you "can't take the flag from anywhere"... You would need to use flag() only the first time or something... Very complicated.

On 20/10/2013, at 01:17, "Marc G." notifications@github.com wrote:

It seems clear from the name that you have to get the flag from something else. Well that's what valerij and I understood at least. I don't understand how it could activate a flag if it didn't actually take it first.

— Reply to this email directly or view it on GitHub.

Mouvedia commented 10 years ago

unflag(foo of !@this) is only one case, the selector can take anything.

I don't think it's a good idea to have a function which has 2 behaviors. Unless we rename it to something that makes sense for both cases. Maybe you want switchFlag() cause that would enable the behaviour you want. It covers both meanings of activation and replacement.

veosotano commented 10 years ago

Why do you do always do all this hair-splitting? We renamed the function on your behest (it was captureFlag at the beginning, remember?), now you say the name doesn't sound what it does...

This function was proposed to do those two functions in the first place. It's really useful because it greatly simplifies a very common task, which is switching active elements in a menu.

So, stop it already.

On 20/10/2013, at 12:36, "Marc G." notifications@github.com wrote:

unflag(foo of !@this) is only one case, the selector can take anything.

I don't think it's a good idea to have a function which has 2 behaviors. Unless we rename it to something that makes sense for both cases. Maybe you want switchFlag() cause that would enable the behaviour you want.

— Reply to this email directly or view it on GitHub.

Mouvedia commented 10 years ago

which is switching active elements in a menu.

Exactly my point.

Capture would have the same problem. I discovered that behaviour you are saying was intended by testing the function and thought it was a bug. Also you could try asking around you, you will see that this behaviour is not expected if you are using that name.

So I propose that this ticket be repurposed to "rename takeFlag()". And my proposal is switchFlag() which meanings would cover all behaviours adequately.