Skip to content

Commit

Permalink
fix: formatting deflayers not working within includes (#17)
Browse files Browse the repository at this point in the history
also: make deflayer formatter enabled by default
  • Loading branch information
rszyma authored Jan 31, 2024
1 parent b71f08b commit 664eeb0
Show file tree
Hide file tree
Showing 8 changed files with 492 additions and 223 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
### Unreleased

* Fixed formatter crash ([#15](https://github.com/rszyma/vscode-kanata/issues/15))

* Make formatter work within included files ([#14](https://github.com/rszyma/vscode-kanata/issues/14))
* Enable formatter by default

### 0.7.0

* Added experimental support for formatting `deflayer`s according to `defsrc` layout (disabled by default, can be enabled in settings)
Expand Down
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ Also, if you work with multiple main files, and find yourself switching `mainCon
there's a handy command palette entry:
- `Kanata: Set current file as main`

### Formatter: auto-apply spacial layout of `defsrc` to all `deflayer`s

This is enabled by default, because I've seen a lot of kanata configs, and it seems like
majority of users prefer to align their `deflayer`s according to spacial layout of `defsrc`.
If you have "Auto format on save enabled" and don't want this feature, you can disable
it in settings (search for "kanata.format").

## Contributing

If you have an idea what could be improved, feel free to open an issue or a PR.
Expand Down
202 changes: 202 additions & 0 deletions kls/src/formatter/defsrc_layout/get_layout.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
use super::{parse_into_ext_tree_and_root_span, ExtParseTree};
use crate::{path_to_url, WorkspaceOptions};
use anyhow::{anyhow, Ok};
use lsp_types::{TextDocumentItem, Url};
use std::{collections::BTreeMap, iter, path::PathBuf, str::FromStr};

pub fn get_defsrc_layout(
workspace_options: &WorkspaceOptions,
documents: &BTreeMap<Url, TextDocumentItem>,
tab_size: u32,
file_uri: &Url, // of current file
tree: &ExtParseTree, // of current file
) -> anyhow::Result<Option<Vec<Vec<usize>>>> {
match workspace_options {
WorkspaceOptions::Single { .. } => {
if tree.includes()?.is_empty() {
tree.defsrc_layout(tab_size)
} else {
// This is an error, because we don't know if those included files
// and current file collectively don't contain >=2 `defsrc` blocks.
// And if that's the case, we don't want to format `deflayers`.
Err(anyhow!("includes are not supported in Single mode"))
}
}
WorkspaceOptions::Workspace {
main_config_file,
root,
} => {
let main_config_file_path = PathBuf::from_str(main_config_file)
.map_err(|e| anyhow!("main_config_file is an invalid path: {}", e))?;
let main_config_file_url = path_to_url(&main_config_file_path, root)
.map_err(|e| anyhow!("failed to convert main_config_file_path to url: {}", e))?;

// Check if currently opened file is the main file.
let main_tree: ExtParseTree = if main_config_file_url == *file_uri {
tree.clone() // TODO: prevent clone
} else {
// Currently opened file is non-main file, it's probably an included file.
let text = &documents
.get(&main_config_file_url)
.map(|doc| &doc.text)
.ok_or_else(|| {
anyhow!(
"included file is not present in the workspace: {}",
file_uri.to_string()
)
})?;

parse_into_ext_tree_and_root_span(text)
.map(|x| x.0)
.map_err(|e| anyhow!("parse_into_ext_tree_and_root_span failed: {}", e.msg))?
};

let includes = main_tree
.includes()
.map_err(|e| anyhow!("workspace [main = {main_config_file_url}]: {e}"))?
.iter()
.map(|path| path_to_url(path, root))
.collect::<anyhow::Result<Vec<_>>>()
.map_err(|e| anyhow!("path_to_url: {e}"))?;

// make sure that all includes collectively contain only 1 defsrc
let mut defsrc_layout = None;
for file_url in includes.iter().chain(iter::once(&main_config_file_url)) {
let text = &documents
.get(file_url)
.expect("document should be cached")
.text;

let tree = parse_into_ext_tree_and_root_span(text)
.map(|x| x.0)
.map_err(|e| {
anyhow!(
"parse_into_ext_tree_and_root_span failed for file '{file_uri}': {}",
e.msg
)
})?;

if let Some(layout) = tree
.defsrc_layout(tab_size)
.map_err(|e| anyhow!("tree.defsrc_layout for '{file_url}' failed: {e}"))?
{
defsrc_layout = Some(layout);
}
}
Ok(defsrc_layout)
}
}
}

#[cfg(test)]
mod tests {
use super::*;

const MAIN_FILE: &str = "main.kbd";

fn new_btree(items: &[(&str, &str)]) -> BTreeMap<Url, TextDocumentItem> {
let mut btree = BTreeMap::new();
for item in items {
let uri = Url::from_str(&format!("file:///{}", item.0)).unwrap();
let doc = TextDocumentItem {
uri: uri.clone(),
language_id: "kanata".to_string(),
version: 0,
text: item.1.to_string(),
};
btree.insert(uri, doc);
}
btree
}

#[test]
fn single_without_includes() {
let src = "(defsrc 1 2) (deflayer base 3 4)";
let layout = get_defsrc_layout(
&WorkspaceOptions::Single { root: None },
&BTreeMap::new(),
4,
&Url::from_str(&format!("file:///{MAIN_FILE}")).unwrap(),
&parse_into_ext_tree_and_root_span(src).unwrap().0,
)
.unwrap()
.ok_or("should be some")
.unwrap();

assert_eq!(layout, vec![vec![2], vec![1]]);
}

#[test]
fn single_with_includes() {
let src = "(defsrc 1 2) (deflayer base 3 4) (include file.kbd)";

let _ = get_defsrc_layout(
&WorkspaceOptions::Single { root: None },
&BTreeMap::new(),
4,
&Url::from_str(&format!("file:///{MAIN_FILE}")).unwrap(),
&parse_into_ext_tree_and_root_span(src).unwrap().0,
)
.expect_err("should be error, because includes don't work in Single mode");

let _ = get_defsrc_layout(
&WorkspaceOptions::Single {
root: Some(Url::from_str("file:///").unwrap()),
},
&BTreeMap::new(),
4,
&Url::from_str(&format!("file:///{MAIN_FILE}")).unwrap(),
&parse_into_ext_tree_and_root_span(src).unwrap().0,
)
.expect_err(
"should be error, because includes don't work in Single mode,\
even if opened in workspace",
);
}

#[test]
fn layout_of_main_file_in_workspace_with_included_defsrc() {
let items = &[
(MAIN_FILE, "(deflayer base 3 4) (include included.kbd)"),
("included.kbd", "(defsrc 1 2) (deflayer numbers 3 4)"),
];
let layout = get_defsrc_layout(
&WorkspaceOptions::Workspace {
main_config_file: MAIN_FILE.to_owned(),
root: Url::from_str("file:///").unwrap(),
},
&new_btree(items),
4,
&Url::from_str(&format!("file:///{MAIN_FILE}")).unwrap(),
&parse_into_ext_tree_and_root_span(items[0].1).unwrap().0,
)
.unwrap()
.ok_or("should be some")
.unwrap();

assert_eq!(layout, vec![vec![3], vec![1]]);
}

#[test]
fn layout_of_included_file_in_workspace_with_included_defsrc() {
let items = &[
(MAIN_FILE, "(deflayer base 3 4) (include included.kbd)"),
("included.kbd", "(defsrc 1 2) (deflayer numbers 3 4)"),
];
let layout = get_defsrc_layout(
&WorkspaceOptions::Workspace {
main_config_file: MAIN_FILE.to_owned(),
root: Url::from_str("file:///").unwrap(),
},
&new_btree(items),
4,
&Url::from_str(&format!("file:///{}", items[1].0)).unwrap(),
&parse_into_ext_tree_and_root_span(items[1].1).unwrap().0,
)
.unwrap()
.ok_or("should be some")
.unwrap();

assert_eq!(layout, vec![vec![3], vec![1]]);
}
}
Loading

0 comments on commit 664eeb0

Please sign in to comment.