Add @fluent/prettier-plugin package to autoformat Fluent resources#668
Add @fluent/prettier-plugin package to autoformat Fluent resources#668wbolster wants to merge 1 commit into
Conversation
|
hi @eemeli, please have a look! i tried to follow project conventions as much as possible. all tests and packaging pass locally, and also in github actions in my own fork. note that i chose 0.1.0 as the version number, and the changelog (which i presume is maintained manually?) reflects that. |
|
fwiw, here's a direct link to the readme, which makes it easier to read: https://github.com/wbolster/fluent.js/blob/prettier-plugin/fluent-prettier-plugin/README.md |
|
paging @Demivan (oh hi 👋🏼) b/c this prettier plugin enhances fluent-vue with formatting inside |
|
the prettier docs about plugin development may be useful for reviewers. |
7d72bd6 to
52b705a
Compare
eemeli
left a comment
There was a problem hiding this comment.
Let's hold off on jumping to a solution right away -- it's only been a few days since the discussion on this even started. As I mention in #667 (comment), we should work towards having just one Prettier plugin for Fluent.
One additional reason I'd prefer to have @luca-iachini on board with the work here is to ensure that any new code we might add to this repo will also be maintained, and that's always easier with multiple maintainers.
|
sure, to keep the discussion centralized i've answered here: #667 (comment) |
| function sortResource(resource: fluentSyntax.Resource): fluentSyntax.Resource { | ||
| type SortableEntry = fluentSyntax.Message | fluentSyntax.Term; | ||
| function compare(a: SortableEntry, b: SortableEntry): number { | ||
| return a.id.name.localeCompare(b.id.name); |
There was a problem hiding this comment.
Should this call toLocaleLowerCase on the id name? I would prefer it.
There was a problem hiding this comment.
sure, i'm open to changing that. but now i wonder in general whether this would need a fixed locale, b/c as i understand it localeCompare() will fallback to the ‘current’ (whatever that may be) locale, which is nondeterministic.
perhaps this can use the locale und (undefined language, e.g. new Intl.Locale('und')) instead of hardcoding (some variation of) english.
wdyt?
There was a problem hiding this comment.
oh yeah, good call. i don't remember all of the subtleties with choosing locales. we could also "just" choose "en", as it's (sadly) the default language in most cases, but maybe that will have other negatives i've not thought of yet.
There was a problem hiding this comment.
it seems locale-agnostic case-insensitive deterministic sort can be achieved using toLowerCase() (not toLocaleLowercase()) and plain > for string comparisons. see e.g. https://stackoverflow.com/questions/68785888/deterministic-string-compare-in-js-ts
i've amended this pull request to use this sort function instead (and added test coverage for it):
function compare(a: SortableEntry, b: SortableEntry): number {
// Compare entry identifiers, ignoring letter case. For
// deterministic results, this deliberately does not use
// locale-aware comparisons (e.g. localeCompare()) and case
// mappings (e.g. toLocaleLowercase()).
const nameA = a.id.name.toLowerCase();
const nameB = b.id.name.toLowerCase();
if (nameA === nameB) return 0;
return nameA > nameB ? 1 : -1;
}lgty @NoahTheDuke?
This adds a @fluent/prettier-plugin subpackage containing a Prettier plugin to autoformat FTL resource files. See added README for details. The packaging, metadata, license, documentation, etc. follows the conventions used by the the other packages in this monorepo. The package is not included in the typedoc docs, because it is not intended to be used as a library, but indirectly via Prettier as a plugin. The vitest configuration now also runs tests written in TypeScript, and @types/node is added as dev dependency, because it gets referenced from tsconfig. Closes projectfluent#667.
52b705a to
c63abf2
Compare
This adds a @fluent/prettier-plugin subpackage containing a Prettier plugin to autoformat FTL resource files. See added README for details.
The packaging, metadata, license, documentation, etc. follows the conventions used by the the other packages in this monorepo. The package is not included in the typedoc docs, because it is not intended to be used as a library, but indirectly via Prettier as a plugin.
The vitest configuration now also runs tests written in TypeScript, and @types/node is added as dev dependency, because it gets referenced from tsconfig.
Closes #667.