[pve-devel] [PATCH proxmox-widget-toolkit] form: add Proxmox.form.field.DisplayEdit

[Dominik Csapak]

first i want to say that i do not really care about
whether we use alias or xtype, but i guess we both
want it to be consistent, and i tried to use xtype for new code in the past

On 4/2/20 5:40 PM, Thomas Lamprecht wrote:
> On 4/2/20 10:26 AM, Dominik Csapak wrote:
>>
>>>>
>>>
>>> Any reason for that? It evaluates to the same after all.. xtype is just an
>>> alias with prefixed "widget."
>>>
>>
>> it's a bit shorter
> 
> Same line count and both well under 80 characters, so I really do not think that
> accounts for anything here.

ok, i just found it always weird to type instead of xtype

> 
>> clearer about what it does (namely setting the xtype)
>>    it is not immediatly clear that setting the alias to 'widget.foo'
>>    results in an xtype of 'foo'
> 
> So I looked a bit into the current use.
> We have 35 uses of "alias" in widget toolkit and 183 in pve-manager.
> I hope it is clear to people working on the gui as else they will run into
> serious issues.

i only get 28 for widget-toolkit and 178 for manager, did you also count
the non widget aliases? not, that it greatly changes the numbers..

yes it's clear after looking at it a bit, but i distinctly remember
being confused at this in the beginning^^

> 
> A heuristic search for xtype use with `ag -A 5  'Ext.define'|ag --stats xtype`
> shows 7 matches in widget toolkit and 60 in manager, so 23.5% using that and
> 76.5% using alias.

ok, in pmg-gui it's about

xtype: 62
alias: 52

so much more percentage of xtype than manager/wt
(having a short look at this, most of the new code
uses xtype, and code copied from manager uses alias)

> 
> I mean, I could also imagine the "xtype" is confusing, as it's normally
> used to instantiate an widget of that type, not defining that the widget
> type name is that.
> 
>> we tend to use it more in newer code
>>
> 
> Who's "we" here? Most recent additions where just a copy+adapt job, so
> it's whatever the source of the initial copy used.

at the time we ported from ext4 to ext6, manuel and i tried to use
xtype in files where we had to touch the context anyway, and also
in new code, so i used xtype since then

> 
> If you want to define and enforce such things, well fine, but they should
> be documented, similar to our Perl Style Guide and ideally some consensus.
> https://pve.proxmox.com/wiki/Perl_Style_Guide
> 
> And then we need to move the >75% of definitions using alias over to xtype,
> or vice versa, move the ~25% of xtype's over to aliases.
> 
> Either is fine to me, but it should be the result of a well documented style
> guide, not just some feeling because one used that more recently, especially
> if it's clearly used by the minority of the code base.
> 
> A basic style guide for JS with ExtJS as big section in there would be nice
> since a long time, care to start one - doesn't have to be really long
> initially..
> 

i am all for having a style guide, i can start one next week after
finishing with the first version of the linter ruleset