Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Refactor i18n middleware language finders #957

Merged
merged 10 commits into from
Mar 22, 2018

Conversation

stanislas-m
Copy link
Member

@stanislas-m stanislas-m commented Mar 6, 2018

  • Split language finders into separate functions
  • Refactor LanguageFinder type
  • Generalize Language finders options
  • Add a new path prefix language finding strategy
  • Ensure setups with default config won't break

Related to: gobuffalo/docs#133

@markbates markbates added this to the 0.11.1 milestone Mar 6, 2018
@markbates
Copy link
Member

@stanislas-m this appears to be a breaking change, how do you propose handling that? via buffalo update?

@stanislas-m
Copy link
Member Author

@markbates buffalo update seems to be the obvious way, yeah. This should only affect projects with a customized i18n middleware (I don't think they are legion), but we can at least give some help for those cases.

@markbates
Copy link
Member

The bare minimum we add to buffalo update should at least detect they’re using stuff that changed and print a warning that they should change things. The nice to have would be doing the code changes for them, if possible.

@stanislas-m
Copy link
Member Author

buffalo update should take care of these cases:

T.CookieName = "language" // Becomes T.LanguageFinderOptions["CookieName"] = "language"
T.SessionName = "language" // Becomes T.LanguageFinderOptions["SessionName"] = "language"
// This one needs refactor:
T.LanguageFinder = func(t *i18n.Translator, c buffalo.Context) []string {
	langs := make([]string, 0)
	if cookie, err := c.Request().Cookie(t.CookieName); err == nil {
		if cookie.Value != "" {
			langs = append(langs, cookie.Value)
		}
	}
	langs = append(langs, t.DefaultLanguage)
	return langs
}

@stanislas-m stanislas-m force-pushed the refactor-i18n-language-finders branch from ef75e1d to 3750d95 Compare March 9, 2018 10:15
@markbates
Copy link
Member

@stanislas-m can you make the changes needed to buffalo update to make sure this doesn't affect too many people?

@stanislas-m
Copy link
Member Author

@markbates Sure, it's on my todo list. :)

* Split language finders into separate functions
* Refactor LanguageFinder type
* Generalize Language finders options
* Add a new path prefix language finding strategy
* Ensure setups with default config won't break
@stanislas-m stanislas-m force-pushed the refactor-i18n-language-finders branch from eabbbdb to 3b4246b Compare March 12, 2018 12:39
@stanislas-m stanislas-m changed the title Refactor i18n middleware language finders WIP - Refactor i18n middleware language finders Mar 12, 2018
@stanislas-m stanislas-m changed the title WIP - Refactor i18n middleware language finders Refactor i18n middleware language finders Mar 12, 2018
@stanislas-m
Copy link
Member Author

stanislas-m commented Mar 12, 2018

@markbates The update works on my side, but a specific case like the following will break (since I've removed the CookieName field):

T.LanguageFinder = func(t *i18n.Translator, c buffalo.Context) []string {
	langs := make([]string, 0)
	if cookie, err := c.Request().Cookie(t.CookieName); err == nil {
		if cookie.Value != "" {
			langs = append(langs, cookie.Value)
		}
	}
	langs = append(langs, t.DefaultLanguage)
	return langs
}

I issue a warning for this one with the update command, is it enough?

@@ -14,27 +14,32 @@ import (
"github.com/pkg/errors"
)

// LanguageFinder can be implemented for custom finding of search
// LanguageFinder - deprecated
Copy link
Member

Choose a reason for hiding this comment

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

what should they use instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just added more infos in the last commit, thanks!

@markbates markbates merged commit 69e1938 into development Mar 22, 2018
@markbates markbates deleted the refactor-i18n-language-finders branch March 22, 2018 18:31
stanislas-m added a commit that referenced this pull request May 12, 2018
* Refactor i18n middleware language finders

* Split language finders into separate functions
* Refactor LanguageFinder type
* Generalize Language finders options
* Add a new path prefix language finding strategy
* Ensure setups with default config won't break

* Handle deprecations

* Fix deprecations replacement

* Add more infos about i18n deprecations
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants