Iterate over catalogs to find the english index and then use it instead of 0.

Codecov Report

Merging #648 into master will increase coverage by 0.23%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #648      +/-   ##
==========================================
+ Coverage   35.11%   35.35%   +0.23%     
==========================================
  Files          68       68              
  Lines        7943     7915      -28     
  Branches     1889     1894       +5     
==========================================
+ Hits         2789     2798       +9     
+ Misses       4374     4341      -33     
+ Partials      780      776       -4

Codecov Report

Merging #648 into master will increase coverage by 0.23%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #648      +/-   ##
==========================================
+ Coverage   35.11%   35.35%   +0.23%     
==========================================
  Files          68       68              
  Lines        7943     7915      -28     
  Branches     1889     1894       +5     
==========================================
+ Hits         2789     2798       +9     
+ Misses       4374     4341      -33     
+ Partials      780      776       -4

igalic (Migrated from github.com) approved these changes 1 year ago

please ignore my rambling

plume-front/src/main.rs

												
			@@ -47,2 +47,4 @@
		
			        let lang = js! { return navigator.language }.into_string().unwrap();
		
			        let lang = lang.splitn(2, '-').next().unwrap_or("en");
		
			        let english_position = catalogs

Plume_migration_agent commented 1 year ago

this should be either a (cached?) constant, or a (cached?) config value

plume-front/src/main.rs

												
			@@ -49,0 +50,4 @@
		
			        let english_position = catalogs
		
			            .iter()
		
			            .position(|(language_code, _)| *language_code == "en")
		
			            .unwrap();

Plume_migration_agent commented 1 year ago

while i understand that this is a simple operation, i don’t think we need to do it every time

Plume_migration_agent commented 1 year ago

actually, never mind, this is an operation that’s only dealing with things known at compile time

the optimiser will probably inline this as a constant literal

requiem958 (Migrated from github.com) reviewed 1 year ago

plume-front/src/main.rs

												
			@@ -47,2 +47,4 @@
		
			        let lang = js! { return navigator.language }.into_string().unwrap();
		
			        let lang = lang.splitn(2, '-').next().unwrap_or("en");
		
			        let english_position = catalogs

Plume_migration_agent commented 1 year ago

It should be a constant yes, but the rust doc of constant says :

Constants can be set only to a constant expression and not to the result of a function call or any other value that will be computed at runtime.

So i thought the keyword const was forbidden in this case.

trinity-1686a reviewed 1 year ago

plume-front/src/main.rs

												
			@@ -47,2 +47,4 @@
		
			        let lang = js! { return navigator.language }.into_string().unwrap();
		
			        let lang = lang.splitn(2, '-').next().unwrap_or("en");
		
			        let english_position = catalogs

trinity-1686a commented 1 year ago

All this is in a lazy_static! call, so the result will be evaluated only one time, the first time CATALOG is accessed. The only thing it’s not is cached between two different page loading, but that’s not necessary imo

Reviewers

Plume_migration_agent approved these changes 1 year ago

The pull request has been merged as 935d331e97.

Plume_migration_agent

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

#648 Fix issue #642, frontend not in english if the user language does not exist

Codecov Report

Codecov Report

Reviewers