Molten icon indicating copy to clipboard operation
Molten copied to clipboard

API

Open LeopoldArkham opened this issue 6 years ago • 7 comments

Discuss what features the API should have.

As far as implementing it, we should start with:

  • Removal of elements
  • Simple appending of values at the end of containers
  • Simple builders for said values

Any code that implements this needs to be vigorously tested as it's obviously a fragile part of the library.

#7 should be looked into before this

Whishlist later on includes:

  • Insertion at any index, or before/after the specified item
  • Full control over trivia such as comments, separators and indentation
  • Sorting of containers
  • Auto indent
  • Ability to canonicalize the file
  • ...?

LeopoldArkham avatar Oct 19 '17 14:10 LeopoldArkham

This would start by converting all raw string references to Cow

LeopoldArkham avatar Nov 16 '17 12:11 LeopoldArkham

Oh, @LeopoldArkham, it was a grave error to ask me for suggestions… but, well, here they come:

I assume there are already methods (planned) to append an entry to a table in a specific format (index doesn't matter, I kinda want cargo add to always append at the end), and to delete an entry as well. That's basically all cargo-edit needs. My goal with cargo add is to add a dependency with the smallest diff possible.

My ultimate goal is to do that in a way that still preserves the overall style of the file. Say, you are using short-and-sweet foo = { version = "0.1", features = ["bar", "baz"] } lines everywhere; I want cargo add lorem-ipsum --features "dolor" to insert this new dependency as inline table. If, on the other hand, you are for some reason using full tables most of the time, cargo add sit-amet --optional should probably be written as a table. In the same manner, we'd also want to keep the formatting of inline-tables (spacing around braces, etc.) consistent.

This is a ludicrous idea, I know! Luckily, a Cargo.toml is a pretty straightforward file, we are mostly dealing with the style of tables, and a hit rate of 60% is probably way better than probably anyone would expect. And maybe someone wants to write their Master's thesis on heuristics for text-based formats or something? I don't know. I've been thinking about this stuff for far too long without actually having time to implement this is a nice way.

In the meantime: Forget the last two paragraphs. I'm absolutely fine with a simple API that allows me to add a line (string or inline table with decent formatting) to a table without fucking up all the comments or whitespace someone added to their file :)

killercup avatar Nov 27 '17 22:11 killercup

Ooh! I really want to see if I can do that!

That said, if we want the user to be able to decide how the file looks, we'll need some equivalent of the ability to just add a string (preferably after parsing it and making sure there are no naming conflicts), since the computer is sure to get it wrong sometimes. So it's definitely worth doing the simpler version first.

jeremydavis519 avatar Nov 28 '17 03:11 jeremydavis519

It should be fairly trivial to read existing entries in the document, see if they're Table or InlineTable (the API provides this) and act accordingly. You'll also be able to convert a Table to InlineTable easily for this purpose. As far as matching the internal style of the spacing, etc, it's a bit more complicated, but I have an idea (basically copy an existing line and re-populate it without touching the style; Not sure if this should be done on the Molten or cargo-edit side, though).

LeopoldArkham avatar Nov 28 '17 16:11 LeopoldArkham

Duplicating is a good idea, @LeopoldArkham! I'm a bit worried about getting a good heuristic to choose what style attributes to copy, i.e., choose a good style when the file is not consistent, and ignoring really weird formatting, or comments inside of the duplicate.

But yeah, let's do the simplest version first and work from there. I'm sure you want to keep Molten's API as small and general-purpose as possible, so it's fine if cargo-edit needs to do more stuff.

killercup avatar Nov 30 '17 11:11 killercup

After a quick look at the Molten's source code, I see a couple of blocking issues for using it in cargo-edit (modulo bugs), @LeopoldArkham, please, correct me if I'm wrong:

  1. All literals are references to a string, representing a TOML document. In cargo-edit we throw away that string. A possible solution would be to use something like owning ref. Also, in order to create a new non-static string literal Item, we would need a Cow string representation.
  2. Key for a table's index is its raw header, so, e.g. in
[a.  "b" . c]

the key will be a. "b" . c, not a, b and c. It means, that if we ask for a key a, we will get nothing. In cargo-edit we need an API for iterating over table's subtables and we want to treat "dependencies" and dependencies keys as equals. A possible solution would be to create a struct

struct Index<'a> {
    doc: &TOMLDocument<'a>,
    path: RefCell<Vec<&'a str>>, // index["a"]["b"] -> vec!["a", "b"]
}

and implement std::ops::Index for it, and probably other methods (and, similarly, IndexMut struct).

Also, I've found a couple of bugs (should probably create a separate issue):

  • Molten can't parse this table: ['dependencies'].
  • (not sure if this is a bug) For this document:
[[bin]] # comment 1
[dependencies]
[[bin]] # comment 2

Molten will map key bin only to second [[bin]] entry.

ordian avatar Dec 21 '17 16:12 ordian

Thanks for your comments, @ordian!

Quoted/dotted keys are not fully implemented yet, that's a known issue, same with non-contiguous AoTs.

  1. is a silly oversight on my part though; Focused on removing allocs and didn't stop to think the source string might not have the same scope. It could always be returned along with the parsed TOMLDoc but that's ugly... I'll look into owning-ref.

LeopoldArkham avatar Dec 23 '17 19:12 LeopoldArkham