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

Enforce nextcloud code style | Adding eslint linting #554

Merged
merged 6 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"extends": [
"@nextcloud"
],
"ignorePatterns": ["dist"],
"overrides": [
{
"files": ["tests/*.js"],
"rules": {
"n/no-unpublished-import": "off",
// Todo: convert tests to TS then drop overrides below
"n/no-missing-import": "off",
"import/no-unresolved": "off",
"import/extensions": "off"
}
},
{
"files": ["*.config.*js"],
"rules": {
"n/no-unpublished-import": "off"
}
}
]
Comment on lines +6 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be amazing to upstream that to the esling config 🚀

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"cypress/**/*.js", "cypress/**/*.ts", "*.cy.js", "*.cy.ts" too (cypress)
"*.config.*js", "*.config.*ts"

}
62 changes: 62 additions & 0 deletions .github/workflows/lint-eslint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# This workflow is provided via the organization template repository
#
# https://github.com/nextcloud/.github
# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization
#
# Use lint-eslint together with lint-eslint-when-unrelated to make eslint a required check for GitHub actions
# https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/troubleshooting-required-status-checks#handling-skipped-but-required-checks

name: Lint

on:
pull_request:
paths:
- '.github/workflows/**'
- 'src/**'
- 'appinfo/info.xml'
- 'package.json'
- 'package-lock.json'
- 'tsconfig.json'
- '.eslintrc.*'
- '.eslintignore'
- '**.js'
- '**.ts'
- '**.vue'

permissions:
contents: read

concurrency:
group: lint-eslint-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

jobs:
lint:
runs-on: ubuntu-latest

name: eslint

steps:
- name: Checkout
uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3

- name: Read package.json node and npm engines version
uses: skjnldsv/read-package-engines-version-actions@1bdcee71fa343c46b18dc6aceffb4cd1e35209c6 # v1.2
id: versions
with:
fallbackNode: '^16'
fallbackNpm: '^7'

- name: Set up node ${{ steps.versions.outputs.nodeVersion }}
uses: actions/setup-node@8c91899e586c5b171469028077307d293428b516 # v3
with:
node-version: ${{ steps.versions.outputs.nodeVersion }}

- name: Set up npm ${{ steps.versions.outputs.npmVersion }}
run: npm i -g npm@"${{ steps.versions.outputs.npmVersion }}"

- name: Install dependencies
run: npm ci

- name: Lint
run: npm run lint
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = {
'ts-jest', {
tsconfig: 'tests/tsconfig.json',
},
]
],
},
transformIgnorePatterns: [],
}
2 changes: 1 addition & 1 deletion lib/date.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="@nextcloud/typings" />

declare var window: Nextcloud.v24.WindowWithGlobals
declare let window: Nextcloud.v24.WindowWithGlobals

/**
* Get the first day of the week
Expand Down
157 changes: 85 additions & 72 deletions lib/gettext.ts
Original file line number Diff line number Diff line change
@@ -1,56 +1,54 @@
/**
* This module provides functionality to translate applications independent from Nextcloud
*
*
* @packageDocumentation
* @module @nextcloud/l10n/gettext
* @example
* ```js
import { getGettextBuilder } from '@nextcloud/l10n/gettext'

const gt = getGettextBuilder()
.detectLocale() // or use setLanguage()
.addTranslation(/* ... *\/)
.build()

.detectLocale() // or use setLanguage()
.addTranslation(/* ... *\/)
.build()
gt.gettext('some string to translate')
```
*/
import GetText from "node-gettext"
import GetText from 'node-gettext'

import { getLanguage } from "."
import { getLanguage } from '.'

/**
* @notExported
*/
class GettextBuilder {

private locale?: string
private translations = {}
private debug = false
private locale?: string
private translations = {}
private debug = false

setLanguage(language: string): GettextBuilder {
this.locale = language
return this
}
setLanguage(language: string): GettextBuilder {
this.locale = language
return this
}

/** Try to detect locale from context with `en` as fallback value */
detectLocale(): GettextBuilder {
return this.setLanguage(getLanguage().replace('-', '_'))
}
/** Try to detect locale from context with `en` as fallback value */
detectLocale(): GettextBuilder {
return this.setLanguage(getLanguage().replace('-', '_'))
}

addTranslation(language: string, data: any): GettextBuilder {
this.translations[language] = data
return this
}
addTranslation(language: string, data: object): GettextBuilder {
this.translations[language] = data
return this
}

enableDebugMode(): GettextBuilder {
this.debug = true
return this
}
enableDebugMode(): GettextBuilder {
this.debug = true
return this
}

build(): GettextWrapper {
return new GettextWrapper(this.locale || 'en', this.translations, this.debug)
}
build(): GettextWrapper {
return new GettextWrapper(this.locale || 'en', this.translations, this.debug)
}

}

Expand All @@ -59,50 +57,65 @@ class GettextBuilder {
*/
class GettextWrapper {

private gt: GetText

constructor(locale: string, data: any, debug: boolean) {
this.gt = new GetText({
debug,
sourceLocale: 'en',
})

for (let key in data) {
this.gt.addTranslations(key, 'messages', data[key])
}

this.gt.setLocale(locale)
}

private subtitudePlaceholders(translated: string, vars: Record<string, string | number>): string {
return translated.replace(/{([^{}]*)}/g, (a, b) => {
const r = vars[b]
if (typeof r === 'string' || typeof r === 'number') {
return r.toString()
} else {
return a
}
})
}

/** Get translated string (singular form), optionally with placeholders */
gettext(original: string, placeholders: Record<string, string | number> = {}): string {
return this.subtitudePlaceholders(
this.gt.gettext(original),
placeholders
)
}

/** Get translated string with plural forms */
ngettext(singular: string, plural: string, count: number, placeholders: Record<string, string | number> = {}): string {
return this.subtitudePlaceholders(
this.gt.ngettext(singular, plural, count).replace(/%n/g, count.toString()),
placeholders
)
}
private gt: GetText

constructor(locale: string, data: object, debug: boolean) {
this.gt = new GetText({
debug,
sourceLocale: 'en',
})

for (const key in data) {
this.gt.addTranslations(key, 'messages', data[key])
}

this.gt.setLocale(locale)
}

private subtitudePlaceholders(translated: string, vars: Record<string, string | number>): string {
return translated.replace(/{([^{}]*)}/g, (a, b) => {
const r = vars[b]
if (typeof r === 'string' || typeof r === 'number') {
return r.toString()
} else {
return a
}
})
}

/**
* Get translated string (singular form), optionally with placeholders
*
* @param original original string to translate
* @param placeholders map of placeholder key to value
*/
gettext(original: string, placeholders: Record<string, string | number> = {}): string {
return this.subtitudePlaceholders(
this.gt.gettext(original),
placeholders
)
}

/**
* Get translated string with plural forms
*
* @param singular Singular text form
* @param plural Plural text form to be used if `count` requires it
* @param count The number to insert into the text
* @param placeholders optional map of placeholder key to value
*/
ngettext(singular: string, plural: string, count: number, placeholders: Record<string, string | number> = {}): string {
return this.subtitudePlaceholders(
this.gt.ngettext(singular, plural, count).replace(/%n/g, count.toString()),
placeholders
)
}

}

/**
* Create a new GettextBuilder instance
*/
export function getGettextBuilder() {
return new GettextBuilder()
return new GettextBuilder()
}
3 changes: 1 addition & 2 deletions lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
/**
* This module provides all functions for the `OC.L10N` namespace
*
*
* @packageDocumentation
* @module @nextcloud/l10n
* @example
* ```js
import { translate as t, translatePlural as n } from '@nextcloud/l10n'

console.log(t('my-app', 'Hello {name}', { name: 'J. Doe' }));
const count = 2;
console.warn(n('my-app', 'Got an error', 'Got multiple errors', 2));
Expand Down
16 changes: 11 additions & 5 deletions lib/registry.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export type Translations = Record<string, string | undefined>
export type PluralFunction = (number: number) => number

declare var window: {
declare let window: {
_oc_l10n_registry_translations: Record<string, Translations>
_oc_l10n_registry_plural_functions: Record<string, PluralFunction>
}
Expand All @@ -13,18 +13,20 @@ interface AppTranslations {

/**
* Check if translations and plural function are set for given app
*
* @param {string} appId the app id
* @return {boolean}
*/
export function hasAppTranslations(appId: string) {
return (
window._oc_l10n_registry_translations?.[appId] !== undefined &&
window._oc_l10n_registry_plural_functions?.[appId] !== undefined
window._oc_l10n_registry_translations?.[appId] !== undefined
&& window._oc_l10n_registry_plural_functions?.[appId] !== undefined
)
}

/**
* Register new, or extend available, translations for an app
*
* @param {string} appId the app id
* @param {object} translations the translations list
* @param {Function} pluralFunction the plural function
Expand All @@ -43,6 +45,7 @@ export function registerAppTranslations(

/**
* Unregister all translations and plural function for given app
*
* @param {string} appId the app id
*/
export function unregisterAppTranslations(appId: string) {
Expand All @@ -52,13 +55,14 @@ export function unregisterAppTranslations(appId: string) {

/**
* Get translations bundle for given app and current locale
*
* @param {string} appId the app id
* @return {object}
*/
export function getAppTranslations(appId: string): AppTranslations {
if (
typeof window._oc_l10n_registry_translations === 'undefined' ||
typeof window._oc_l10n_registry_plural_functions === 'undefined'
typeof window._oc_l10n_registry_translations === 'undefined'
|| typeof window._oc_l10n_registry_plural_functions === 'undefined'
) {
console.warn('No OC L10N registry found')
return {
Expand All @@ -75,6 +79,7 @@ export function getAppTranslations(appId: string): AppTranslations {

/**
* Set new translations and plural function for an app
*
* @param {string} appId the app id
* @param {object} translations the translations list
* @param {Function} pluralFunction the plural function
Expand All @@ -90,6 +95,7 @@ function setAppTranslations(

/**
* Extend translations for an app
*
* @param {string} appId the app id
* @param {object} translations the translations list
* @param {Function} [pluralFunction] the plural function (will override old value if given)
Expand Down
Loading