files-community / Files

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

Code Quality: Generative resource management and refactoring #15662

Open XTorLukas opened 1 week ago

XTorLukas commented 1 week ago

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Generated Strings properties from keys in primary Resources file
  2. Compatible import from .resw and .json file
0x5bfa commented 2 days ago

Is this still on-development? If you are ok, I’d like to review.

0x5bfa commented 1 day ago

Could you duplicate ResourceHelpers in Strings.cs in Helpers namespace?

Sample impl:

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

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

Sample usage (not for now):

<TextBlock Text="{helpers:Strings Name=Allow}" />
XTorLukas commented 1 day ago
<TextBlock Text="{helpers:Strings Name=Allow}" />

I would use instead of Name=, Key=

  <TextBlock Text="{helpers:Strings Key=Allow}" />
XTorLukas commented 1 day ago
[MarkupExtensionReturnType(ReturnType = typeof(string))]
public partial class Strings : MarkupExtension
{
    public string Name { get; set; }

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

This way it would be usable, you would just have to introduce a rule into the generator that key names e.g. 'Key' or 'Name' cannot be used because they are used internally. Evaluating with an analyzer would be good.

[MarkupExtensionReturnType(ReturnType = typeof(string))]
public sealed partial class Strings : MarkupExtension
{
    public string Key { get; set; } = default!;

    protected override string ProvideValue()
        => GetType().GetField(Key)?.GetValue(this) as string ?? string.Empty;
}