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

margin/align name inconsistency #145

Closed
Xeverous opened this issue Jun 10, 2020 · 11 comments
Closed

margin/align name inconsistency #145

Xeverous opened this issue Jun 10, 2020 · 11 comments

Comments

@Xeverous
Copy link
Contributor

I just realized there is align_left, align_right etc but also left_margin, right_margin. Is this intended? IMO it would be good to have everything spelled one way.

@djowel
Copy link
Member

djowel commented Jun 10, 2020

Good point. What do you suggest? margin_right sounds kinda awkward. left_align sounds OK. Either way, this change will involve a lot of work on current apps using it. How about supporting both synonyms?

@Xeverous
Copy link
Contributor Author

I have mixed feelings about supporting both. thing_left, thing_right sounds most reasonably for me because most code is going from most significant to least significant (eg namespace::sub_namespace::func, struct.inner_struct.member, element.margin.top = ..., element.margin.right = ...).

@djowel
Copy link
Member

djowel commented Jun 10, 2020

OK, but how about vtile, htile, etc.? Also, in english, you call it "right margin" (a noun) No one calls it "margin right", unless it is a phrase: "margin at the right". So is align-left, "align" is not a noun, it is a verb. So that's the difference. align_left is basically a shortcut of the phrase "align the element to the left". "alignment" is a noun, and so "left alignment" can be a long-cut of "left align". What can I say, it is english...

@djowel
Copy link
Member

djowel commented Jun 10, 2020

namespace::sub_namespace::func, struct.inner_struct.member, element.margin.top = ..., element.margin.right = ...).

It would probably be cool to have this syntax: margin.right, align.left, align.left.top

@djowel
Copy link
Member

djowel commented Jun 10, 2020

namespace::sub_namespace::func, struct.inner_struct.member, element.margin.top = ..., element.margin.right = ...).

It would probably be cool to have this syntax: margin.right, align.left, align.left.top

Yes, that can be done, I think.

@djowel
Copy link
Member

djowel commented Jun 10, 2020

It would probably be cool to have this syntax: margin.right, align.left, align.left.top

Yes, that can be done, I think.

I think I like this syntax a lot. It also solves my problem of combination. For example, align.left.top should be the same as align.top.left. For margins, it is very intuitive: margin.left(10).right(20) which has the same semantics as margin.right(20).left(10).

I've also been mulling about the alternative syntax for proxies. Examples:

box | align.top.left | margin(10, 10, 20, 20)

... compared to the current syntax:

align_top_left(margin(10, 10, 20, 20), box)

@Xeverous
Copy link
Contributor Author

I really like |. Let's go functional. I see a lof of potential library design space to be explored, and such element transformations are much more readable than f1(f2(f3(elem))).

@djowel
Copy link
Member

djowel commented Jun 10, 2020

Now that more users are already using elements, if we do this, it is time to use the [[deprecated]] feature. It is also probably time to have a stabilized version, on the way to v1.0 (perhaps v0.8). My idea is to move deprecated APIs in a separate directory, to keep the main API clean. Any API that is bound to change, that is already in the examples or already documented, should have a deprecation notice.

@Xeverous
Copy link
Contributor Author

New thing: docs mention:

// variant 1
vmargin({ top, bottom }, subject)

// variant 2
vmargin(top, bottom, subject)

// variant 3
top_bottom_margin({ top, bottom }, subject)

// variant 4
top_bottom_margin(top, bottom, subject)

But actually none of these functions exist. There are xside_margin and yside_margin instead.

@djowel
Copy link
Member

djowel commented Aug 18, 2020

Good catch! I'll look into it ASAP.

@djowel djowel mentioned this issue Aug 26, 2020
15 tasks
@djowel
Copy link
Member

djowel commented Feb 10, 2024

fixed in e2c2d22

@djowel djowel closed this as completed Feb 10, 2024
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

No branches or pull requests

2 participants