files-community / Files

Building the best file manager for Windows
https://files.community
MIT License
33k stars 2.11k forks source link

Code Quality: Generative resource management using Source Generator #15504

Open 0x5bfa opened 1 month ago

0x5bfa commented 1 month ago

Description

Instead of pasting raw string code of resources let’s have a source generator to use properties. The source generator will update the class right after resw file changed.

Concerned code

None. Making new robust code.

Gains

Requirements

App.Strings.Strings partial class.

If comments start with GENERIC or something we can apart the class into Strings.Generic, Strings.Infecrectable, Strings.Settings, etc. this is just in my mind and not official proposal tho.

Here’s code:

// Strings.Inflectsble.cs
/// <auto-generated/>
public partial class Strings
{
    /// <summary>
    /// (generated comments from resw here)
    /// </summary>
    public string AddNewItem
        => “Add new items”;
}
Strings.AddNewItems.GetLocalized(itemCount);

If we want to make localization methods simplify, let's make Strings.Extension class file as partial.

// Strings.Extension.cs
[MarkupExtensionReturnType(ReturnType = typeof(string))]
public partial class Strings : MarkupExtension
{
    public string Name { get; set; }

    protected override object ProvideValue()
        => GetType().GetProperty(Name)?.GetValue(this);
}

Comments

No response

0x5bfa commented 1 month ago

No one reacts…

@XTorLukas fyi

0x5bfa commented 1 month ago

If there’s slash the SG avoids to generate it, since we’re in a long term to reduce number of strings that contains it.

XTorLukas commented 1 month ago

Yes I agree that a plain RAW string is not quite suitable for finding errors and editing later. I would also omit the slash. But splitting would be useful e.g. Strings.KeyDelDescription.GetLocalized() --> Key: "Key.Del.Description" Value: "Del". Possible splitting according to a predefined parser that replaces 'Key' with 'Key.' ... This would not use a slash in user space but in the background of the code

XTorLukas commented 1 month ago

Generating a class using resx would certainly be useful, but creating it would not be trivial and the subsequent implementation would have to be on the git side. If you dare, I can possibly help you with creating a parser and implementing it in the project

0x5bfa commented 1 month ago

Then replace it with underscore.

0x5bfa commented 1 month ago

Generating a class using resx would certainly be useful, but creating it would not be trivial and the subsequent implementation would have to be on the git side. If you dare, I can possibly help you with creating a parser and implementing it in the project

I guess Source Generator has features specifically for this kind of use case. CsWin32 uses it.

XTorLukas commented 1 month ago

Generating a class using resx would certainly be useful, but creating it would not be trivial and the subsequent implementation would have to be on the git side. If you dare, I can possibly help you with creating a parser and implementing it in the project

I guess Source Generator has features specifically for this kind of use case. CsWin32 uses it.

Unfortunately I don't know this, I haven't worked with this tool yet

0x5bfa commented 1 month ago

Oh ok!

@yaira2 what do you think? Like CsWin32 does we can generate string properties right after rests changed.

XTorLukas commented 1 month ago

Then replace it with underscore.

Maybe, switch to '_', is OK.

XTorLukas commented 1 month ago

@0x5bfa I had an idea of how values could be approached.

Type for keys

This would make it possible to group common keys

 public class StringsStandartBase(string key)
 {
     public string Key => key;
     public string Localized(params object[] pairs) => StringsResourceLocalized.GetLocalized(Key, pairs);
 }

 public class StringsExtendedBase(string key, string title, string description) : StringsStandartBase(key)
 {
     public StringsStandartBase Title => new(title);
     public StringsStandartBase Description => new(description);
 }

Strings

// Strings.Inflectsble.cs
/// <auto-generated/>
public partial class Strings
{
    /// <summary>
    /// (generated comments from resw here)
    /// </summary>
    public static StringsStandartBase AddItem => new("AddItemKey");
    ...
    /// <summary>
    /// (generated comments from resw here)
    /// </summary>
    public static StringsExtendedBase ActionAddItem => new("AddItem", "AddItem_Title", "AddItem_Description");
    ...
}

Use in code

Strings.AddItem.Localized(itemCount);
Strings.ActionAddItem.Localized(itemCount);
Strings.ActionAddItem.Title.Localized(itemCount);
Strings.ActionAddItem.Description.Localized(itemCount);

What do you think of this concept? It seemed like a faster solution that would allow access by group.


Also, it would be good to somehow solve how many values can be used after the placeholders for the format, i.e. {0, number} {1, number} ,, then 2x must be used i.e. Localized(itemCount1, itemCount2). If used incorrectly, this now raises an Exception which is ignored and the result is then string.Empty.

0x5bfa commented 1 month ago

You sure this is faster? It looks promising and richer but I’m not against that idea. If @yaira2 thinks it’s good we can move forward with my concept.

XTorLukas commented 1 month ago

This is faster because all common keys are grouped under one object. I also thought of using ReadOnlySpan instead of string, but then we would have to use ref struct instead of class This speeds up access to data

0x5bfa commented 1 month ago

By the way tree yaml supported in Crowdin? And comments too?

XTorLukas commented 1 month ago

yes it supports both image image from this data: https://github.com/XTorLukas/Files/blob/305fb4c3d56dd819fca85e4d9f0f7b6b9ac88fa5/src/Files.App.Resources/Strings/en-US/Resources.yml

0x5bfa commented 1 month ago

Wow!! Comments can be place right above of the resource?

XTorLukas commented 1 month ago

Wow!! Comments can be place right above of the resource?

Also why not, just they must always be written without the space e.i. "#Comments"

XTorLukas commented 1 month ago

I did a complex migration from RESW to YML, here I placed all comments above the values by default https://github.com/XTorLukas/Files/tree/xtorlukas/TestImplementYamlResourceManager/src/Files.App.Resources/Strings Now using RESW to JSON

XTorLukas commented 1 month ago

My idea would be this:

Structure for key string holder

public ref struct StringsStandartBase
{
    public required ReadOnlySpan<char> Key { get; init; }

    public static implicit operator StringsStandartBase(string key)
        => new() { Key = key };
}

Structure for key string holder with specified properties

public ref struct StringsActionBase
{
    public readonly required StringsStandartBase Title { get; init; }
    public readonly required StringsStandartBase Description { get; init; }

    public static implicit operator StringsActionBase((string title, string description) keys)
        => new() { Title = keys.title, Description = keys.description };
}

Extensions for Unicode formatter

public static class StringsExtensions
{
    public static string ToLocalized(this StringsStandartBase keyValue, params object[] pairs) => string.Empty;
}

AutoGenerated class

public partial class Strings
{
    public static StringsStandartBase AddItem => "AddItemKey";
    public static StringsStandartBase SpecKey => "SpecKey";

    public static StringsActionBase ActionAddItem => ("Title", "Description");
}

Use in code

 string value;
 value = Strings.ActionAddItem.Title.ToLocalized();
 value = Strings.ActionAddItem.Description.ToLocalized();
 value = Strings.AddItem.ToLocalized();
0x5bfa commented 4 weeks ago

This is faster because all common keys are grouped under one object.

I mean, wouldn’t it too rich for strings to instance a class just for strings? In my opinion, string types sufficiently well do.