class library: add File *read #2410

telephon · 2016-10-15T19:24:21Z

Instead of requiring a callback function, it is much clearer to just
read from file and return the value.

Instead of requiring a callback function, it is much clearer to just read from file and return the value.

crucialfelix · 2016-10-20T07:04:26Z

HelpSource/Classes/File.schelp

+open the file, evaluate the function with the file as argument, and close it again. If the process fails, close the file and throw an error.
+
+method::read
+given a path, make a new temporary file, and read the contents, close the file and return them. By default, it calls link::#-readAllString::. If the process fails, close the file and throw an error.


huh ? so you create a temporary file and then read the contents. That suggests that you just read an empty file. You just created it, then you read it. It is empty.

Then you close it and return "them" (who is them ? oh.. the contents)

ah, you are not creating a temporary file at all ! You just mean that internally you make a File object that does the reading. That is an implementation detail that the user should not be told about. The person calling this method does not need to know about the internals. Just the arguments and what is returned.

"make a temporary file" means this: https://www.google.de/search?hl=en&source=hp&q=make+a+temporary+file&btnG=Google-Suche&oq=&gws_rd=cr&ei=XmwIWMPPH-XX6ASXvaOIAQ

good point, I just meant the file object!

crucialfelix · 2016-10-20T07:06:11Z

HelpSource/Classes/File.schelp

+the path (link::Classes/String::) that points to the file
+
+argument::mode
+the file mode to use.


Only valid modes are "r" and "rb", correct ?

yes. It may be argued that we don't need this option, but I don't know.

… I see you mean, I should add this information. Yes, I'll do so asap.

crucialfelix · 2016-10-20T07:08:50Z

HelpSource/Classes/File.schelp

+argument::selector
+the method to be called on the file once it is open.
+
+


What are the valid methods that could be supplied here ? \readAllString , ... ?

crucialfelix · 2016-10-20T07:10:23Z

Also it would be good to start each sentence with a capital letter.

crucialfelix · 2016-10-20T17:15:25Z

looks great now, thanks !

nhthn · 2016-10-20T19:16:23Z

SCClassLibrary/Common/Files/File.sc

@@ -18,6 +18,11 @@ File : UnixFILE {
 		file = this.new(pathName, mode);
 		^{ function.value(file) }.protect({ file.close });
 	}
+	*read { arg pathName, mode = "r", selector = \readAllString;


How about methodName instead of selector?

selector is already a convention from the various perform methods.

Understood.

yes, it's a notion that comes from the smalltalk tradition. A symbol selects a method from the receiving object's vocabulary.

Used in ObjectiveC as well:

https://developer.apple.com/library/content/documentation/General/Conceptual/DevPedia-CocoaCore/Selector.html

.

vivid-synth · 2016-10-21T15:23:31Z

HelpSource/Classes/File.schelp

@@ -40,12 +40,35 @@ File.getcwd;
 ::

 method::use
-open the file, evaluate the function with the file and close it.
+Open the file, evaluate the function with the file as argument, and close it again. If the process fails, close the file and throw an error.


Wording nitpick: the tense here ("If the process fails, close the file and...") can be a bit confusing. Beginners may wonder if that's an instruction for something they need to do. I'd prefer the wording for e.g. mode ("Opens a file" vs "Open a file")

(By the way, this already is inconsistent throughout this file -- e.g. mkdir vs delete)

I agree on being painstakingly nitpicking with expressing things! The "traditional" standard in sc helpfiles is e.g. for round "Round to multiple of aNumber". Or sign "Answer -1 if negative, +1 if positive or 0 if zero." Or min "the minimum of the receiver and aMagnitude." But if we feel strongly about this, it can be changed of course, but this may mean many small changes …

I also agree that in this case it sound a bit like an instruction for the user! I'll change that.

vivid-synth · 2016-10-21T15:23:43Z

HelpSource/Classes/File.schelp

+Open the file, evaluate the function with the file as argument, and close it again. If the process fails, close the file and throw an error.
+
+method::read
+Given a path for an existing file, read and return the contents. By default, it calls link::#-readAllString::. If the process fails, close the file and throw an error.


Same note as above

adcxyz · 2016-12-05T21:54:41Z

I agree that file opening and closing is boring admin for users.
Maybe the clearest option would be to have separate methods for all 'read entire file' methods?
Assuming text files are the default case, that could be:

File.read("path/to/file.txt");
File.readRTF("path/to/file.rtf");
File.readHTML("path/to/file.html");
File.readSignalFile.readSignal("path/to/file.sig");

Then users need not worry about read modes, cant erase files by accident etc.

For simple formatted text files, there is also the FileReader class and its subclasses.

nhthn · 2017-05-14T02:15:17Z

I like this. Is it ready for merge?

telephon · 2017-05-14T16:41:37Z

I wonder what you think about @adcxyz 's comments. It seems like a viable simplification.

mossheim · 2017-05-14T16:55:28Z

I prefer @adcxyz's suggestion; it matches the API of the existing "readAllX" instance methods.

There is now a helpfile conflict with master, but should be trivial to resolve it.

telephon · 2017-05-14T18:36:42Z

agreed, I'll do these.

telephon · 2017-05-14T19:37:35Z

I've come up with these two simple methods:

*readString { arg pathName;
		var res;
		this.use(pathName, "r", { |file| res = file.readAllString });
		^res
}
*readSignal { arg pathName;
		var res;
		this.use(pathName, "r", { |file| res = file.readAllSignal });
		^res
}

If you want to strip away rtf, you can write:

File.readString(path).stripRTF

Does that look good?

mossheim · 2017-05-14T19:41:58Z

Why not rtf and html?

telephon · 2017-05-14T19:45:25Z

The interface readRTF had its purpose when the help system was still in RTF, same for HTML. So I just thought it's not really so important. Also semantically stripRTF seemed better, says what it does. It doesn't really give you a RTF representation back.

mossheim · 2017-05-14T19:58:17Z

OTOH, it costs nothing to also make convenience functions for HTML and RTF, and this provides a more complete and consistent interface. I have personally used SC to read/strip HTML. "it's not really so important" seems like an odd argument to use given the triviality of these functions.

telephon · 2017-05-15T15:06:22Z

probably it's harmless to just exactly replicate the instance read interface. For a moment I was put off by the fact that instead of just reading, it strips.

telephon · 2017-05-16T20:36:05Z

@brianlheim @adcxyz here is a new version, let me know if you like it, then I update the help.

The interface now simply matches the one of the instance methods:

       *readAllString { arg pathName;
		var res;
		this.use(pathName, "r", { |file| res = file.readAllString });
		^res
	}
	*readAllSignal { arg pathName;
		var res;
		this.use(pathName, "r", { |file| res = file.readAllSignal });
		^res
	}
	*readAllStringHTML { arg pathName;
		var res;
		this.use(pathName, "r", { |file| res = file.readAllStringHTML });
		^res
	}
	*readAllStringRTF { arg pathName;
		var res;
		this.use(pathName, "r", { |file| res = file.readAllStringRTF });
		^res
	}

mossheim · 2017-05-16T21:35:58Z

@telephon lgtm, thanks!

While `res` is a good generic name for a response or result, here `string` is better.

nhthn

any objections to this? aside from the conflict, it looks fine to me.

mossheim · 2017-05-24T23:34:25Z

Help needs to be updated, and then I propose we merge :) Thanks!

telephon · 2017-05-25T06:24:58Z

I tried to resolve it in the online editor, but somehow my browser doesn't show the GUI properly so I couldn't save the change. Maybe someone else can try?

mossheim · 2017-05-25T17:19:20Z

Still need to update the help file... it's referencing a non-existent method now.

nhthn · 2017-05-25T18:51:14Z

i'll take care of it

class library: easier file access

7d18d63

Instead of requiring a callback function, it is much clearer to just read from file and return the value.

telephon added enhancement comp: class library SC class library labels Oct 15, 2016

telephon added this to the future milestone Oct 15, 2016

crucialfelix reviewed Oct 20, 2016

View reviewed changes

crucialfelix changed the title ~~class library: easier file access~~ class library: add File-read Oct 20, 2016

crucialfelix changed the title ~~class library: add File-read~~ class library: add File *read Oct 20, 2016

telephon added 3 commits October 20, 2016 17:53

help: improve method doc for File *read

63197c2

help: File: capitalize sentences

b2a243e

help: add File read example

9551276

nhthn previously requested changes Oct 20, 2016

View reviewed changes

nhthn approved these changes Oct 20, 2016

View reviewed changes

vivid-synth reviewed Oct 21, 2016

View reviewed changes

telephon self-assigned this May 14, 2017

nhthn added the waiting for consensus label May 14, 2017

class library: File read classmethods match instance methods

ba3cbe5

class library: File read methods, better variable name

a889b9b

While `res` is a good generic name for a response or result, here `string` is better.

nhthn approved these changes May 24, 2017

View reviewed changes

Merge branch 'master' into topic-easier-file-access

0f74082

telephon merged commit 17e132a into supercollider:master May 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

class library: add File *read #2410

class library: add File *read #2410

telephon commented Oct 15, 2016

crucialfelix Oct 20, 2016 •

edited

Loading

telephon Oct 20, 2016

crucialfelix Oct 20, 2016

telephon Oct 20, 2016

telephon Oct 20, 2016

crucialfelix Oct 20, 2016

crucialfelix commented Oct 20, 2016

crucialfelix commented Oct 20, 2016

nhthn Oct 20, 2016

jamshark70 Oct 20, 2016

nhthn Oct 20, 2016

telephon Oct 21, 2016

crucialfelix May 15, 2017

vivid-synth Oct 21, 2016

telephon Oct 21, 2016 •

edited

Loading

vivid-synth Oct 21, 2016

adcxyz commented Dec 5, 2016 •

edited by telephon

Loading

nhthn commented May 14, 2017

telephon commented May 14, 2017

mossheim commented May 14, 2017

telephon commented May 14, 2017

telephon commented May 14, 2017 •

edited

Loading

mossheim commented May 14, 2017

telephon commented May 14, 2017

mossheim commented May 14, 2017

telephon commented May 15, 2017

telephon commented May 16, 2017 •

edited

Loading

mossheim commented May 16, 2017

nhthn left a comment

mossheim commented May 24, 2017

telephon commented May 25, 2017

mossheim commented May 25, 2017

nhthn commented May 25, 2017

		argument::selector
		the method to be called on the file once it is open.

class library: add File *read #2410

class library: add File *read #2410

Conversation

telephon commented Oct 15, 2016

crucialfelix Oct 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crucialfelix commented Oct 20, 2016

crucialfelix commented Oct 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

telephon Oct 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adcxyz commented Dec 5, 2016 • edited by telephon Loading

nhthn commented May 14, 2017

telephon commented May 14, 2017

mossheim commented May 14, 2017

telephon commented May 14, 2017

telephon commented May 14, 2017 • edited Loading

mossheim commented May 14, 2017

telephon commented May 14, 2017

mossheim commented May 14, 2017

telephon commented May 15, 2017

telephon commented May 16, 2017 • edited Loading

mossheim commented May 16, 2017

nhthn left a comment

Choose a reason for hiding this comment

mossheim commented May 24, 2017

telephon commented May 25, 2017

mossheim commented May 25, 2017

nhthn commented May 25, 2017

crucialfelix Oct 20, 2016 •

edited

Loading

telephon Oct 21, 2016 •

edited

Loading

adcxyz commented Dec 5, 2016 •

edited by telephon

Loading

telephon commented May 14, 2017 •

edited

Loading

telephon commented May 16, 2017 •

edited

Loading