Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic implementation for Alignment #150

Closed
karljj1 opened this issue Apr 9, 2021 · 9 comments
Closed

Generic implementation for Alignment #150

karljj1 opened this issue Apr 9, 2021 · 9 comments
Labels
Milestone

Comments

@karljj1
Copy link
Collaborator

karljj1 commented Apr 9, 2021

Hi,
Im looking at one of the examples in the code:
Placeholder.cs has this example:

"{Items.Length,10:choose(1,2,3):one|two|three}"

For some reason the alignment is not being applied. It looks like its only done in the Default Formatter and its not being applied in this example for me. Any ideas? Also I had to change it to use | instead of , for the format values.

@karljj1 karljj1 changed the title Example does not work as expected Example does not work as expected - Alignment ignored Apr 9, 2021
@axunonb axunonb closed this as completed in 87b4865 Apr 9, 2021
@axunonb
Copy link
Member

axunonb commented Apr 9, 2021

Thanks for the hint.
The examples in xmldoc should of course be correct. But you're better off having a look at the unit tests, simply because they all must work.

@axunonb axunonb reopened this Apr 9, 2021
@axunonb
Copy link
Member

axunonb commented Apr 9, 2021

If a custom formatter is used, the default formatter will not be called. That's why there's no alignment.
Taking care of Alignment must take place in the custom formatter.
Wonder whether this brings any benefit for the standard ChooseFormatter.

@axunonb
Copy link
Member

axunonb commented Apr 15, 2021

Should Alignment become part of IFormatter?
Then it was clear this must be tackled by a formatter (throw NotImplementedException, do nothing, deal like string.Format).

@axunonb axunonb changed the title Example does not work as expected - Alignment ignored Make Alignment part of IFormatter Apr 15, 2021
@karljj1
Copy link
Collaborator Author

karljj1 commented Apr 15, 2021

Should Alignment become part of IFormatter?
Then it was clear this must be tackled by a formatter (throw NotImplementedException, do nothing, deal like string.Format).

Yeah it makes sense to do that. Its confusing to a user at the moment as its only applied when 1 type of formatter is used.
Im trying to write our docs to explain to a non-programmer how to format strings and it confused me ;)

@axunonb
Copy link
Member

axunonb commented Apr 15, 2021

Still thinking about what's better to add to IFormatter?

void AlignFormat();
or
Action<int, char> AlignFormat { get; set; } (here, the fill character instead of blank could become part of the implementation)

Feature for v3.

@karljj1
Copy link
Collaborator Author

karljj1 commented Apr 15, 2021

Isn't it essentially the same thing for all of them? If the formatter handles the item then apply the alignment. Maybe it could be handled in a generic way so that it does not need to be implemented by each formatter?

@axunonb
Copy link
Member

axunonb commented Apr 15, 2021

Will consider this option, too. Definitely a feature request.

@axunonb axunonb changed the title Make Alignment part of IFormatter Make Alignment part of IFormatters Apr 15, 2021
@axunonb axunonb added this to the version/v3 milestone Apr 27, 2021
@axunonb axunonb changed the title Make Alignment part of IFormatters Generic implementation for Alignment Jun 3, 2021
@axunonb
Copy link
Member

axunonb commented Jun 3, 2021

version/v3:
Handling alignment will go to FormattingInfo, while SmartFormatter.EvaluateSelectors will set FormattingInfo.Alignment from the parsed Format. This way, alignment will be available to all formatters out-of-the-box. The character used to align items defaults to space (0x20), but can be customized with FormatterSettings.AlignmentFillCharacter.

@axunonb
Copy link
Member

axunonb commented Jun 4, 2021

Closed with "Alignment option always available" (#174)

@axunonb axunonb closed this as completed Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants