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

fix: Schemes with *.yml extension is not recognized #99

Merged
merged 8 commits into from
Jan 21, 2025
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixed

- Fixed a bug where scheme files ending in `.yml` weren't recognized.

## [0.26.0] - 2025-01-18

### Added
Expand Down
1 change: 0 additions & 1 deletion src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@ pub const SCHEMES_REPO_NAME: &str = "schemes";
pub const CUSTOM_SCHEMES_DIR_NAME: &str = "custom-schemes";
pub const CURRENT_SCHEME_FILE_NAME: &str = "current_scheme";
pub const DEFAULT_SCHEME_SYSTEM: &str = "base16";
pub const SCHEME_EXTENSION: &str = "yaml";
pub const SCHEMES_REPO_REVISION: &str = "spec-0.11";
36 changes: 25 additions & 11 deletions src/operations/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,20 +171,34 @@ impl Lightness {
fn as_json(scheme_files: HashMap<String, SchemeFile>) -> Result<String> {
let mut keys: Vec<String> = scheme_files.keys().cloned().collect();
// Create a thread-safe HashMap to collect results
let locked_results = Arc::new(Mutex::new(HashMap::new()));
let mutex = Arc::new(Mutex::new(HashMap::new()));
let mut sorted_results: Vec<SchemeEntry> = Vec::new();
// We could be parsing hundreds of files. Parallelize with 10 files each arm.
keys.par_chunks(10).try_for_each(|chunk| -> Result<()> {
for key in chunk {
if let Some(scheme) = scheme_files.get(key).and_then(|sf| sf.get_scheme().ok()) {
let mut results_lock = locked_results.lock().unwrap();
results_lock.insert(key.clone(), SchemeEntry::from_scheme(&scheme));
scheme_files
.into_iter()
.collect::<Vec<_>>()
// We could be parsing hundreds of files. Parallelize with 10 files each arm.
.par_chunks(10)
.map(|chunk| {
chunk
.into_iter()
.filter_map(|(k, sf)| {
sf.get_scheme()
.ok()
.map(|scheme| (k.to_string(), SchemeEntry::from_scheme(&scheme)))
})
.collect::<HashMap<String, SchemeEntry>>()
})
.for_each(|map| {
// Each batch will produce a HashMap<String, SchemaFile>
// Merge them into the final HashMap.
if let Ok(mut accum) = mutex.lock() {
accum.extend(map);
}
}
Ok(())
})?;
});

keys.sort();
let results = locked_results.lock().unwrap();
let results = mutex.lock().unwrap();

for k in keys {
if let Some(v) = results.get(&k) {
sorted_results.push(v.clone());
Expand Down
39 changes: 17 additions & 22 deletions src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::config::{Config, ConfigItem, DEFAULT_CONFIG_SHELL};
use crate::constants::{REPO_NAME, SCHEME_EXTENSION};
use crate::constants::REPO_NAME;
use anyhow::{anyhow, Context, Error, Result};
use home::home_dir;
use rand::Rng;
Expand Down Expand Up @@ -392,32 +392,27 @@ pub fn get_all_scheme_file_paths(
continue;
}

for file in fs::read_dir(&scheme_system_dir)? {
let file_path = file.as_ref().unwrap().path();
let extension = file_path
.extension()
.unwrap_or_default()
.to_str()
.unwrap_or_default();

if extension == SCHEME_EXTENSION {
let files = fs::read_dir(&scheme_system_dir)?
// Discard failed read results
.filter_map(|o| o.ok())
.collect::<Vec<_>>()
.into_iter()
.filter_map(|file| {
// Convert batch of files into a HashMap<String, SchemeFile>, where
// the key is the scheme's <system>-<slug> e.g. base16-github
// Map each entry into a (<String, SchemaFile) tuple that
// we can collect() into this batch's HashMap<String, SchemaFile>
let name = format!(
"{}-{}",
scheme_system.as_str(),
file.unwrap()
.path()
.file_stem()
.unwrap_or_default()
.to_str()
.unwrap_or_default()
file.path().file_stem()?.to_str()?,
);

let scheme_file = SchemeFile::new(file_path.as_path())?;
scheme_files.insert(name.clone(), scheme_file);
}
}
let scheme_file = SchemeFile::new(file.path().as_path()).ok()?;
return Some((name, scheme_file));
})
.collect::<HashMap<String, SchemeFile>>();
scheme_files.extend(files);
}

Ok(scheme_files)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/cli_list_subcommand_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn test_cli_list_subcommand_with_custom() -> Result<()> {
"",
)?;
fs::write(
custom_scheme_path.join(format!("{}/{}.yaml", scheme_system, scheme_name_two)),
custom_scheme_path.join(format!("{}/{}.yml", scheme_system, scheme_name_two)),
"",
)?;

Expand Down
Loading