Contribution guidelines for token types

[lczech]

Hi @lyskov,

I'm thinking to contribute to Binder (see #324), but before starting, wanted to quickly check in with you about the contribution guidelines for the project:

Would you accept adding another token prefix for the config file? I think that + and - are not intuitive fits for things such as renaming or return value policy overwrites, and I find it also is weird for the default_... return value policies. How about adding = as a token prefix for things that are "setters"? (For backwards compatibility, the + could be kept as an alternative for the existing ones of course).

Looking forward to hearing your thoughts
Lucas

PS: Minor thing: The current line length of 200 is too wide for comfortably working in my editor (and I guess I'm not the only one). Would you be open to changing this to, say, 100?

[lczech]

Hey again, addition to the above: After having dived into the code a bit now, I have noticed that there are several tokens where only the + variant makes sense, such as prefix_for_static_member_functions. From a syntactical point of view, it thus seems weird to have to require a + or - in front when not all token types fit with that pattern.

If you are open to this, I suggest to refactor this, and change this to a more intuitive pattern, such as add_class and rm_class (or some variant, skip_class, for instance), instead of +class and -class. This could be done in a backwards compatible way, so that existing Binder config files do not need to change (although it would be a simple search-and-replace).

Let me know what you think, and I'm happy to implement this :-)

[lyskov]

PS: Minor thing: The current line length of 200 is too wide for comfortably working in my editor (and I guess I'm not the only one). Would you be open to changing this to, say, 100?

I am open to general idea but could you please done this as separate PR? Also, 100 is rather extreme how about ~150?

[lczech]

Haha didn't mean to open up that can of worms - in the end there is a degree of personal preference of course with no right or wrong answer. Yet, I've never seen any style guide that goes beyond 120, see e.g., here. Mostly it is 80 or 100, sometimes 120. It was quite new to me to see 200 here. Reasons for shorter length are of course side-by-side editors (I'm having 4 panels next to each other), and readability, see also here and here.

Looking at the line length distribution of Binder, there are not many long lines anyway:

So having a wide editor window just for those few lines is not making use of the space. Automatic line wrapping of course works, but isn't nice.

Anyway, your code base. It was just a suggestion, because I've never seen a limit of 200. If you want to keep it, all good.

[lyskov]

re token types: in general yes but only if desired config could not be specified by other means. It will probably be a good idea to open separate issue for each case with examples of current and desired config text so it could be evaluated and agreed upon before you have to spend time coding it.
Thanks,

[lczech]

Makes sense. In summary, my suggestions are as above: The +/- syntax is only semantically meaningful for a few tokens, while most tokes now have to use + because that's what the code expects, but it isn't intuitive for instance why there has to be a + in front of prefix_for_static_member_functions. So, I suggest to refactor this, and use add_class instead of +class etc. That all can be done with full backwards compatibility by having +class be an alias.