Error on `package.path` shorthand

[Kyuuhachi]

Current behavior

If I write package = { path = "../package" }, everything works just fine. But if I instead write package.path = "../package", it gives an error Error fetching crate. I think those two are supposed to be equivalent in TOML — cargo accepts it without complaining, at least.

Does the same if I write for example extend.version = "1.1.0", though that's less important since there's already a convenient shorthand for that.

Expected behavior

Treat the above two snippets as equivalent and don't error.

Neovim version

nvim --version

NVIM v0.8.2
Build type: Release
LuaJIT 2.1.0-beta3
Compiled by builduser

Features: +acl +iconv +tui
See ":help feature-compile"

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/usr/share/nvim"

Run :checkhealth for more info

[saecki]

I'm a bit torn on this one. The way toml files are currently parsed is just a bunch of regexes. This works quite well actually, because we can make a lot of assumptions and be quite accommodating for invalid syntax like unclosed quotes or similar. The downside to it is that we can't realistically handle every syntactically correct way of specifying dependencies. We could implement this additional case, but that would add complexity in many places. Maybe it is a common enough case though, that it would be worth supporting. The "right" solution for this issue would be a lenient toml parser, that can handle any valid toml file you throw at it, and would combine the best of both worlds. A while ago I've dabbled a bit with writing one in rust, but that isn't far along, writing one in lua/teal, would be a bit of a pain and probably too slow to be a nice user experience.

Anyway, I'm currently quite busy and wouldn't have time to implement this myself.

[utkarshgupta137]

@Saecki doesn't Rust already have a TOML parser? I mean if cargo can parse them, then you could just borrow the code?

[saecki]

Yes indeed, there are a lot of TOML parsers. Unfortunately most of them wont be able to recover from errors (they are not lenient). As a result we wouldn't be able to recover any information as soon as one syntax error occurs.

[utkarshgupta137]

Yes, but most of the time we are dealing with valid TOMLs? Would it be possible to use a parser & fall back to your current logic if the parser fails? Even when inserting versions or features, the state would be like this:

serde = ""
# or
serde = { version = "1.0", features = [] }

[saecki]

I would argue the opposite, most of the time we are dealing with invalid TOMLs. When editing a file, most of the time the state is incomplete, and only stop when we archived the wanted result, which is a syntactically valid file. Especially when editing running two parsers at a time would probably mean double the latency, which is undesirable.

[utkarshgupta137]

Idk, in my experience, the TOML itself is usually valid. Operations such as removing crates/features or adding features or changing versions shouldn't break the TOML. It would probably only break if you start from an empty line. I usually copy an existing line & then modify it, so for me, the TOML is rarely broken. Anyway, would it make sense to use parser + regex fallback if not in insert mode & using only regex if in insert mode? I think it would be a pretty nice tradeoff. The biggest issue for me right now is that crates.nvim continues showing an error in NeoTree, even if I close the file. Many times, I open it just to check if crates are included or required features are enabled or not. There is no way to get rid of it without restarting nvim, which takes 10-15 seconds because I have a workspace with 50K+ lines & 800+ deps.

[utkarshgupta137]

Here is the minimum reproduction of the issue I'm facing btw:

==> Cargo.toml <==
[workspace]
members = ["crates1", "crates2"]

[dependencies]
crates1 = { version="0.1", path = "crates1" }
crates2 = { version="0.1", path = "crates2" }

==> crate1 <==

==> crate1/Cargo.toml <==
[package]
name = "crate1"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

==> crate1/src <==

==> crate1/src/lib.rs <==
pub fn add(left: usize, right: usize) -> usize {
    left + right
}

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

    #[test]
    fn it_works() {
        let result = add(2, 2);
        assert_eq!(result, 4);
    }
}

==> crate2 <==

==> crate2/Cargo.toml <==
[package]
name = "crate2"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
crate1 = { workspace=true, features = [] }

==> crate2/src <==

==> crate2/src/lib.rs <==
pub fn add(left: usize, right: usize) -> usize {
    left + right
}

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

    #[test]
    fn it_works() {
        let result = add(2, 2);
        assert_eq!(result, 4);
    }
}

In crate2/Cargo.toml, I'm getting Error fetching crate for crate1. I'm assuming the issue is related to regex parsing too? If not, i can open a separate issue.

[saecki]

I don't think this is related, could you move this to a separate issue?

[saecki]

For anyone interested, I've been working on said parser lately.