sveinungf / spreadcheetah

SpreadCheetah is a high-performance .NET library for generating spreadsheet (Microsoft Excel XLSX) files.
MIT License
294 stars 16 forks source link

Source generator - Reference named styles #44

Closed sveinungf closed 1 month ago

sveinungf commented 6 months ago

Background and motivation

Right now there is no way to use any styling with the source generator. Enabling styling by only using attributes (for example as suggested in #31 and #43) is an approach that would be easy to use, but requires a lot of work (and potentially many attributes) before all parts of styling would be supported.

If there would be a way to first add named styles to a spreadsheet, then these could be referenced with attributes on properties. This would be a way to have all supported forms of styling with the source generator.

API proposal

namespace SpreadCheetah;

public sealed class Spreadsheet
{
    public StyleId AddStyle(Style style, string name);
}
namespace SpreadCheetah.SourceGeneration;

[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public sealed class CellStyleAttribute(string styleName) : Attribute;

API usage

public class Person
{
    [CellStyle("My FirstName style")]
    public string FirstName { get; set; }
}

[WorksheetRow(typeof(Person))]
public partial class PersonRowContext : WorksheetRowContext;
await using var spreadsheet = await Spreadsheet.CreateNewAsync(stream);
await spreadsheet.StartWorksheetAsync("Sheet 1");

var style = new Style();
style.Font.Bold = true;

spreadsheet.AddStyle(style, "My FirstName style");

var person = new Person { FirstName = "Ola" };
await spreadsheet.AddAsRowAsync(person, PersonRowContext.Default.Person);
sveinungf commented 1 month ago

This has been implemented in version 1.17.0.

cajuncoding commented 4 weeks ago

It would be nicer if the style names weren't forced to be String values -- which encourages use of magic strings by devs. But, a custom Enum would be really nice.... you could implement this with a Generic overload that simply uses the .ToString() of whatever is passed in as the Key to method and under the hood continue to use the string based key correllation, but allow Dev teams to create an Enum of styles that makes sense for a report.

sveinungf commented 4 weeks ago

I see your point. However since the named styles also can be shown in the Excel UI, I want to be able to use any character in the name. If the name was based on e.g. an Enum, then having a white-space in the name would be problematic.

If you want to use an Enum instead, perhaps you can do it by adding your own extension method? E.g.:

public static StyleId AddStyle<T>(this Spreadsheet spreadsheet, Style style, T name)
{
    return spreadsheet.AddStyle(style, name.ToString());
}

For the CellStyle attribute you can reference an Enum with nameof:


public class MyClass
{
    [CellStyle(nameof(MyEnum.Bold))]
    public int MyValue { get; set; }
}
cajuncoding commented 3 weeks ago

Yeah. that works, but having to use nameof() isn't ideal.

I was thinking about it some more and realized that the main issue is that your Attribute classes are sealed so we cannot override to augment with our own custom functionality. For example, this same issue exists with Microsoft's own AuthorizeAttribute which takes in Role names as strings (mainly because parameters to attributes have to be valid constant expressions), but by overriding it we can provide our own elegant constructors that take in our Role Enum and the code is nice and tidy; internally we map the Enum to a string value (e.g. role.ToString()).

So what are your thoughts on un-sealing the attributes to make it more flexible for consumers?

[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public MyAppCellStyleAttribute : CellStyleAttribute
{
    public MyAppCellStyleAttribute(CellStyleEnum styleEnum) : base(styleEnum.ToString())
    { }
}
sveinungf commented 3 weeks ago

Unfortunately I don't think it would be trivial to make that work. A source generator only "looks" at the code during compilation, it doesn't execute the code. In your example the source generator could see there is a call to ToString(), but it won't actually call the ToString() method. Maybe it is obvious what ToString() should return in this case, but there wouldn't be anything to prevent you from writing more complex C# code in the attribute. We could then easily end up in a situation where it will be very hard (or maybe impossible) for the source generator to figure out what the resulting string should be.

I would guess deriving from AuthorizeAttribute works because whatever reads that attribute is using reflection and not a source generator. In that case it all happens during runtime, so the runtime can then also execute your custom code.

cajuncoding commented 3 weeks ago

Ahhh, ok yes we haven't embraced using source generators yet so I often don't think about the implications there.

In our current feature I'm working on, we've unfortunately had to stop using the source generation approach in some cases with SpreadCheetah due to the need for more flexible cell styling and dynamic changes for each row.... so I wasn't thinking about source generation. Thanks for the feedback 👍