zwilias / elm-json

Install, upgrade and uninstall Elm dependencies
MIT License
183 stars 9 forks source link

No newline at end of file #23

Closed brandly closed 4 years ago

brandly commented 4 years ago

hi! thanks for this project.

after uninstalling a couple dependencies, git diff reports my changes which include

Screen Shot 2020-04-28 at 12 19 40 PM

it seems like elm produces an elm.json which includes a trailing newline, but this project does not include the newline. maybe i'll dig in and send you a PR

zwilias commented 4 years ago

Cool, that would be appreciated!

brandly commented 4 years ago

hello again! i came back to look at this. i learned some Rust months ago but haven't done much useful with it. i still have lots to learn.

is the correct approach here to implement the Serialize trait for Project rather than deriving it here?

https://github.com/zwilias/elm-json/blob/79ac385c4e13e7f2fd3cb9fc84f6723a93275718/src/lib/project/mod.rs#L13-L18

i still have to figure out how to implement this trait, but i wanted to make sure i'm headed in the right direction. thanks!

harrysarson commented 4 years ago

I think instead it would be better to find the point in the code where the json is written to a file and then add writeln!(&mut writer, ""); afterwards.

brandly commented 4 years ago

thanks @harrysarson! i tried that route, had trouble, and then thought about this Serialize route.

maybe i'm misunderstand, but putting writeln!(&mut writer, ""); on line 63...

https://github.com/zwilias/elm-json/blob/79ac385c4e13e7f2fd3cb9fc84f6723a93275718/src/lib/cli/util.rs#L54-L64

...results in this error

error[E0596]: cannot borrow `writer` as mutable, as it is not declared as mutable
  --> src/lib/cli/util.rs:63:14
   |
57 |     let writer = BufWriter::new(file);
   |         ------ help: consider changing this to be mutable: `mut writer`
...
63 |     writeln!(&mut writer, "");
   |              ^^^^^^^^^^^ cannot borrow as mutable

error[E0382]: borrow of moved value: `writer`
  --> src/lib/cli/util.rs:63:14
   |
57 |     let writer = BufWriter::new(file);
   |         ------ move occurs because `writer` has type `std::io::BufWriter<std::fs::File>`, which does not implement the `Copy` trait
58 |     let formatter = serde_json::ser::PrettyFormatter::with_indent(b"    ");
59 |     let mut serializer = serde_json::Serializer::with_formatter(writer, formatter);
   |                                                                 ------ value moved here
...
63 |     writeln!(&mut writer, "");
   |              ^^^^^^^^^^^ value borrowed here after move

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0382, E0596.
For more information about an error, try `rustc --explain E0382`.
error: could not compile `elm-json`.

i kind of understand ownership but not well enough to see the solution here. my understanding is that the ownership was passed to the serializer, which led me to thinking i should modify how things get serialized.

harrysarson commented 4 years ago

Aha yes, I see the issue. So zwilias created a Writer called writer and moved this writer into serializer on line 59. Exactly like you said l, once something is moved you can no longer access the old value (ownership is passed to serializer).

I still like my idea (but then it is my idea so I am biased). The Serializer struct in serde_json provides a method into_inner which consumes the serializer and gives you back the writer used to create it. To write the new line we first have to get our selves the writer back once we are done specializing and the write to the file.

Hope that helps :) (I should add that I am just an interested observer, not a maintainer or anything so please feel free to ignore me entirely)