[pve-devel] [PATCH proxmox-widget-toolkit] form: add Proxmox.form.field.DisplayEdit
Thomas Lamprecht
t.lamprecht at proxmox.com
Thu Apr 2 17:40:57 CEST 2020
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.
> 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.
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.
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.
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..
More information about the pve-devel
mailing list