Skip to content

Conversation

@erkkikeranen
Copy link
Member

@erkkikeranen erkkikeranen commented May 4, 2020

(slurp "example.txt") file contents
(slurp "doesn't exists.txt") #Condition["No such file or directory (os error 2)"]
(def s (slurp "example.txt")) ; works
(def s (slurp "http://www.example.com"))

Also refactored rust_core to separate files

@erkkikeranen erkkikeranen force-pushed the slurp branch 3 times, most recently from 25ae824 to a87243e Compare May 4, 2020 09:44
@erkkikeranen erkkikeranen changed the title WIP: slurp can read a text file slurp files and http URLs May 4, 2020
@erkkikeranen erkkikeranen marked this pull request as ready for review May 4, 2020 09:47
@Tko1
Copy link
Member

Tko1 commented May 4, 2020

Looks good. I don't know if slurp needs to be in another file -- rust_core, the file, represents that entire namespace of functions, just as clojure/core.clj does. Often it really just depends, I could maybe see us splitting it into its own file -- if we split all the rust_core functions into their own files. But then, they don't each need to be their own module, each their own thing to be imported to be used. Splitting up the files would be more about being able to see more of the structure of rust_core by looking at the file system, to be able to see each function and macro at a glance there (which helps if your editor doesn't easily display the list of functions in the file you're looking at) -- but overall I don't think that's a problem. You might say its also about being able to look at each function in isolation, but because they have one consistent, repeated structure, rust_core can just be seen as a list of fns and macros, and there's a clear delineation between the function definitions -- you can look at each one in isolation pretty easily there.

Anyways, so for now its preferable to have slurp in rust_core.rs

@Tko1
Copy link
Member

Tko1 commented May 4, 2020

As mentioned on discord, the two ways of breaking up the files for rust_core have tradeoffs, and even though I have reasons for preferring the other way, it shouldn't be significant enough to matter -- so to merge this PR request

  1. Add this to rust_core.rs instead
    or
  2. Split up rust_core.rs into files so that it matches this
@erkkikeranen erkkikeranen changed the title slurp files and http URLs slurp files and http URLs, refactor rust_core structure May 5, 2020
@Tko1 Tko1 merged commit b129bc6 into clojure-rs:master May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants