Open Enyium opened 3 weeks ago
Thanks for the suggestion.
I think most of it make sense, and it can indeed be done by adding a new and deprecating the old one. Although i don't think that can be done for trait method
row_data() to row()
Not sure about this one. row()
could be mistaken with the row number.
property getters use that prefix. I understand that using no prefix/suffix at all makes name conflicts with other methods possible that generated types may provide. Maybe Slint could use a prop_ prefix instead?
I think it's fine to break the guideline in this case.
I find some parts of Slint's API to not properly follow Rust's conventions. Here are some things that stood out to me:
MapModel::source_model()
(also on similar types) should just besource()
Model
trait could have more concise names:tracker()
instead ofmodel_tracker()
row_data()
torow()
andset_row_data()
toset_row()
.VecModel
already does in parameter names):FilterModel::unfiltered_row()
becomesFilterModel::unfiltered_index()
,SortModel::unsorted_row()
becomesSortModel::unsorted_index()
.row_count()
could becomelen()
. This would also affect user types that implementModel
and that may not have "Model" in their name. But a model is inherently a list, and user types implementingModel
can be expected to have this flair. There's also already the generally phrasediter()
on the trait. (If required, an alternative would benum_rows()
, which I'd prefer over something with "count".)ModelRc
andVecModel
viadefault()
. But there should also be a parameter-lessnew()
constructor. GPT-4o agrees that, if it makes sense for a type to have a parameter-less constructor, it should be namednew()
. Only if the type needs parameters in every constuctor maynew()
have parameters. Constructors like the currentModelRc::new()
should be namedfrom_...()
(see "conversion constructors" in Rust's API guidelines).VecModel::from_slice()
returns something other than aVecModel
is just inconsistent. Shouldn't users rather call.into()
, like Rust always requires devs to be explicit?get_
prefix is not used for getters in Rust code." But property getters use that prefix. I understand that using no prefix/suffix at all makes name conflicts with other methods possible that generated types may provide. Maybe Slint could use aprop_
prefix instead? This could also be seen as adhering to Rust's philosophy of being explicit. For consistency, it should then probably also beset_prop_...()
.Global
trait has aget()
function to fetch a component's global-singleton instance. The Rust API guidelines say: "Theget
naming is used only when there is a single and obvious thing that could reasonably be gotten by a getter. For exampleCell::get
accesses the content of aCell
."Global::get()
doesn't get a "single and obvious thing", but is dependent on a component. As far as I know,getInstance()
is a popular name in general for the instance getter of a singleton. Rust's avoidance of aget_
prefix would shorten this toinstance()
. If you go after this list of programming abbreviations,inst()
would be a suitable, common abbreviation.To make for a seamless transition, you could duplicate the functions where the ones with the old names call through to the ones with the new names, which would contain the current implementations. The functions with the old names then get a
#[deprecated]
attribute with a message saying what is to be used instead and that the function will be removed in Slint v2. This way, users get warnings with nice messages what to replace what with, while Slint v1 is still the current major version.EDIT:
Timer
also has thedefault()
/new()
problem.no-frame
attribute onWindow
in .slint code should be phrased positively instead of negatively, so maybehas-frame
with a default oftrue
(although built-inhas-...
properties always seem to be out-properties). Positive phrasing should generally be preferred to avoid possible double-negatives.