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

Improve template (axes footer) #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

appgurueu
Copy link
Contributor

The current template is overly verbose (see #39). This PR attempts to alleviate that. Please discuss.

@appgurueu
Copy link
Contributor Author

I've switched methods to :method(...) for headers. Two things I don't like about this:

  • Stuffing the arguments in the header is redundant. They should probably go.
  • Similarly, stuffing the returns in the header is hardly possible (a, b = :method(c, d) is invalid Lua and looks odd; omitting the self doesn't really work if you want to put something in front of the colon).

I'd be fine with switching to :method or .method, but not with removing the : and . as is the case with the current template. Colon vs. dot is very important here since both are possible and used by Minetest (e.g. auth handler doesn't need self and uses dot). We also want to remind newcomers or people coming from other programming languages that they have to pass self as an implicit first argument (without writing it down in the argument table). Simply having methods be in a subsection "methods" is not explicit enough.

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.

None yet

1 participant