Skip to content

Error in the comment for wxfilename and rxfilename #3264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
csukuangfj opened this issue Apr 25, 2019 · 4 comments · Fixed by #3153
Closed

Error in the comment for wxfilename and rxfilename #3264

csukuangfj opened this issue Apr 25, 2019 · 4 comments · Fixed by #3153

Comments

@csukuangfj
Copy link
Contributor

See the comment below

// We now document the types of extended filenames that we use.
//
// A "wxfilename" is an extended filename for writing. It can take three forms:
// (1) Filename: e.g. "/some/filename", "./a/b/c", "c:\Users\dpovey\My
// Documents\\boo"
// (whatever the actual file-system interprets)
// (2) Standard output: "" or "-"
// (3) A pipe: e.g. "gunzip -c /tmp/abc.gz |"
//
//
// A "rxfilename" is an extended filename for reading. It can take four forms:
// (1) An actual filename, whatever the file-system can read, e.g. "/my/file".
// (2) Standard input: "" or "-"
// (3) A pipe: e.g. "| gzip -c > /tmp/abc.gz"
// (4) An offset into a file, e.g.: "/mnt/blah/data/1.ark:24871"
// [these are created by the Table and TableWriter classes; I may also write
// a program that creates them for arbitrary files]
//

For the wxfilename

(3) A pipe: e.g. "gunzip -c /tmp/abc.gz |"

which is actually an rxfilename.

For the rxfilename

(3) A pipe: e.g. "| gzip -c > /tmp/abc.gz"

which is a wxfilename.

@danpovey
Copy link
Contributor

Yes you're right. Please make a PR to fix this along with the other small errors you noticed.

@csukuangfj
Copy link
Contributor Author

In addition, the following lists some misleading names in the source code:


RspecifierType ClassifyRspecifier(const std::string &rspecifier,
std::string *wxfilename,
RspecifierOptions *opts) {

std::string *wxfilename, should be std::string *rxfilename,


void UnitTestClassifyWspecifier() {
{
std::string a = "b,ark:foo|";

Even though ClassifyWspecifier does not check the validity of the string foo|, it is misleading to use an rxfilename here to demonstrate ClassifyWspecifier

@danpovey
Copy link
Contributor

danpovey commented Apr 25, 2019 via email

@csukuangfj
Copy link
Contributor Author

@danpovey
fixed in #3153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants