Wrap program revised.

[name]

Back for more critique.

----------------------------------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

#define MAX 10000

/*
wrap.c inserts newlines in place of spaces according to specified
line length. Output filename is {filename}.wrap. Takes two arguments,
filename and line length.
*/

void wordwrap(FILE *ifp, FILE *ofp, char *wl)
{
char buf[MAX];
int c, i, space, count, length;
length = atoi(wl);
for (i = 0; i < MAX && ((c=getc(ifp)) != EOF); ++i)
buf = (char)c;
for (i = 0; buf != '\0'; ++i) {
if ((i == 0) || ((buf == '\n' || buf == '\t') && buf[i-1] == '\n'))
count = space = 0;
if (buf == ' ')
space = i;
++count;
if ((count == length) && (space != 0)) {
buf[space] = '\n';
count = i - space;
}
}
for (i = 0; buf != '\0'; ++i) {
c = (int)buf;
putc(c, ofp);
}
}

int main(int argc, char *argv[])
{
FILE *fp1;
FILE *fp2;
char *prog = argv[0];
char *filename1 = argv[1];
char filename2[40];
char *wl = argv[2];
if (argc != 3) {
printf("Usage: %s: filename, wrap length\n", prog);
return EXIT_FAILURE;
} else if (strlen(argv[1]) > 32) {
printf("Filename limited to 32 characters. Sorry...\n");
return EXIT_FAILURE;
}
strcpy(filename2, argv[1]);
strcat(filename2, ".wrap");
if (atoi(wl) > 80) {
printf("Line length limit: 80. Better is < 75.\n");
return EXIT_FAILURE;
} else if (atoi(wl) < 0) {
printf("Line length must be a positive number.\n");
return EXIT_FAILURE;
} else if ((fp1 = fopen(filename1, "r")) == NULL) {
fprintf(stderr, "%s: can't open %s\n", prog, filename1);
return EXIT_FAILURE;
} else if ((fp2 = fopen(filename2, "w")) == NULL) {
fprintf(stderr, "%s: can't open %s\n", prog, filename2);
return EXIT_FAILURE;
} else {
printf("Wrapping %s at %s\n", filename1, wl);
printf("Output file adds .wrap to input filename.\n");
wordwrap(fp1, fp2, wl);
fclose(fp1);
fclose(fp2);
}
if (ferror(fp2)) {
fprintf(stderr, "%s: error writing %s\n", prog, filename2);
return EXIT_FAILURE;
}
return EXIT_SUCCESS;
}
-----------------------------------------------------------------

There are some number of different "canonical formats" for C code. This
approximates one of them. I don't find it as readable as the style I use,
but this is for those who didn't like my style. Hope this serves.

Thanks for reading.

 

[Barry Schwarz]

Back for more critique.

----------------------------------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

#define MAX 10000

/*
wrap.c inserts newlines in place of spaces according to specified
line length. Output filename is {filename}.wrap. Takes two arguments,

I see three in the function.

filename and line length.
*/

void wordwrap(FILE *ifp, FILE *ofp, char *wl)
{
char buf[MAX];
int c, i, space, count, length;
length = atoi(wl);
for (i = 0; i < MAX && ((c=getc(ifp)) != EOF); ++i)
buf = (char)c;

This cast serves no practical purpose but may be used to suppress an
annoying and irrelevant diagnostic from the compiler.

for (i = 0; buf != '\0'; ++i) {

When will buf be '\0'. The only time I can see is when the input
file contains a '\0'. Do you expect there to be only one at the end
of the file? I would expect most text files to not have any '\0' at
all. Maybe you want to add the following after the previous Input
loop:
buf = '\0';

if ((i == 0) || ((buf == '\n' || buf == '\t') && buf[i-1] == '\n'))
count = space = 0;

What is your intent here? Why is the sequence \n\n significant? Why
is \n\t significant

if (buf == ' ')
space = i;
++count;
if ((count == length) && (space != 0)) {

If count grows to length but space is still 0 (a single very long
word), you skip the range of this if and on the next iteration of the
loop increment count beyond length. From then on, count can never
equal length. Maybe you want to use >=.

buf[space] = '\n';
count = i - space;
}
}
for (i = 0; buf != '\0'; ++i) {
c = (int)buf;

This cast is useless. In fact, the whole statement accomplishes
nothing. Simply use buf in the following call to putc.

putc(c, ofp);
}
}

int main(int argc, char *argv[])
{
FILE *fp1;
FILE *fp2;
char *prog = argv[0];
char *filename1 = argv[1];
char filename2[40];
char *wl = argv[2];
if (argc != 3) {
printf("Usage: %s: filename, wrap length\n", prog);
return EXIT_FAILURE;
} else if (strlen(argv[1]) > 32) {
printf("Filename limited to 32 characters. Sorry...\n");
return EXIT_FAILURE;
}
strcpy(filename2, argv[1]);
strcat(filename2, ".wrap");
if (atoi(wl) > 80) {
printf("Line length limit: 80. Better is < 75.\n");
return EXIT_FAILURE;
} else if (atoi(wl) < 0) {
printf("Line length must be a positive number.\n");
return EXIT_FAILURE;
} else if ((fp1 = fopen(filename1, "r")) == NULL) {
fprintf(stderr, "%s: can't open %s\n", prog, filename1);
return EXIT_FAILURE;
} else if ((fp2 = fopen(filename2, "w")) == NULL) {
fprintf(stderr, "%s: can't open %s\n", prog, filename2);
return EXIT_FAILURE;

Since each if block ends with a return statement, all the elses,
including the next one, are superfluous. If any if evaluates to true,
none of the following elses will be "executed".

} else {

It may be just me but I admit to hating this particular style.

printf("Wrapping %s at %s\n", filename1, wl);
printf("Output file adds .wrap to input filename.\n");
wordwrap(fp1, fp2, wl);
fclose(fp1);
fclose(fp2);
}
if (ferror(fp2)) {
fprintf(stderr, "%s: error writing %s\n", prog, filename2);
return EXIT_FAILURE;
}
return EXIT_SUCCESS;
}
-----------------------------------------------------------------

There are some number of different "canonical formats" for C code. This
approximates one of them. I don't find it as readable as the style I use,
but this is for those who didn't like my style. Hope this serves.

Thanks for reading.

<<Remove the del for email>>

 

[name]

I see three in the function.

Ummm... yes, including the program name. I'm not sure how this is considered.
Launching the app itself (here, './wrap') invokes the first argument to the
code as argv[0], but is considered a launch with no arguments. Or is this
wrong. I'm talking to the user here who needn't have a clue about the code
itself.

filename and line length.
*/

void wordwrap(FILE *ifp, FILE *ofp, char *wl)
{
char buf[MAX];
int c, i, space, count, length;
length = atoi(wl);
for (i = 0; i < MAX && ((c=getc(ifp)) != EOF); ++i)
buf = (char)c;

This cast serves no practical purpose but may be used to suppress an
annoying and irrelevant diagnostic from the compiler.

An annoying diagnostic from lclint, to be precise. Compiled and worked just
fine without it, but given the depth of criticism lurking hereabouts...

for (i = 0; buf != '\0'; ++i) {

When will buf be '\0'. The only time I can see is when the input
file contains a '\0'. Do you expect there to be only one at the end
of the file? I would expect most text files to not have any '\0' at
all. Maybe you want to add the following after the previous Input
loop:
buf = '\0';

Well, '\0' is the null character at the end of an string by definition, and
I guess I'm treating an ASCII text file as a string. In any case, I'm
reading a file into an array, and for the scale I'm treating at the moment,
it works. So the null character is the check point for EOF. However, that
won't be the general solution.

One of the next steps in this project is to learn all about file types.
Starting with 'man file'... <grin>. When I get a handle on all that, I'll
be able to hack a way of testing files to see what I really need to do.

That's part of the fun and part of this learning experience!

In the meantime, maybe I should simply make it explicit as you suggest.

if ((i == 0) || ((buf == '\n' || buf == '\t') && buf[i-1] == '\n'))
count = space = 0;

What is your intent here? Why is the sequence \n\n significant? Why
is \n\t significant

Ummm.. yes. Well, the fact is that this entire section is too complicated.
A newline should restart the count, period. A newline means a long line is
at the end, and has been wrapped. As many newlines in a row as one might
encounter doesn't change that. Same with a tab, it's safe to assume a tab
is 8 spaces, and if it's less, then the line will be wrapped short. No
problem. And a tab can be encountered anywhere, not just at the beginning
of a line.

So, should be:

if (buf == '\n')
count = space = 0;
if (buf == '\t')
count = count + 8;

Occam's Razor anyone?? Sheesh! LOL!!!

if (buf == ' ')
space = i;
++count;
if ((count == length) && (space != 0)) {

If count grows to length but space is still 0 (a single very long
word), you skip the range of this if and on the next iteration of the
loop increment count beyond length. From then on, count can never
equal length. Maybe you want to use >=.

Hmmm... good point!! I added the second conditional to accommodate Mr.
ODwyer's two 20-x words, but didn't see the result wrt 'count'.

Oh dear... dunno why this just now occured to me, but if a single word
exceeds length, it should be hyphenated. So I have to add that as well, and
that will be difficult because that addresses syllables, which requires some
sort of database or complex algorithm.

Not sure how to even begin thinking about that. But that has to be handled

buf[space] = '\n';
count = i - space;
}
}
for (i = 0; buf != '\0'; ++i) {
c = (int)buf;

This cast is useless. In fact, the whole statement accomplishes
nothing. Simply use buf in the following call to putc.

putc(c, ofp);
}
}

ROFL!!!

Talk about not seeing the obvious!!! I can't think why I didn't do that,
but it occurs to me that there was some reason. Whatever it might have
been, it no longer exists. Works just fine. Away with another set of
braces!!

Thanks!

int main(int argc, char *argv[])
{
FILE *fp1;
FILE *fp2;
char *prog = argv[0];
char *filename1 = argv[1];
char filename2[40];
char *wl = argv[2];
if (argc != 3) {
printf("Usage: %s: filename, wrap length\n", prog);
return EXIT_FAILURE;
} else if (strlen(argv[1]) > 32) {
printf("Filename limited to 32 characters. Sorry...\n");
return EXIT_FAILURE;
}
strcpy(filename2, argv[1]);
strcat(filename2, ".wrap");
if (atoi(wl) > 80) {
printf("Line length limit: 80. Better is < 75.\n");
return EXIT_FAILURE;
} else if (atoi(wl) < 0) {
printf("Line length must be a positive number.\n");
return EXIT_FAILURE;
} else if ((fp1 = fopen(filename1, "r")) == NULL) {
fprintf(stderr, "%s: can't open %s\n", prog, filename1);
return EXIT_FAILURE;
} else if ((fp2 = fopen(filename2, "w")) == NULL) {
fprintf(stderr, "%s: can't open %s\n", prog, filename2);
return EXIT_FAILURE;

Since each if block ends with a return statement, all the elses,
including the next one, are superfluous. If any if evaluates to true,
none of the following elses will be "executed".

Yeah, I know. Somehow I got the notion that this was a well regarded coding
style. Something about an intermidable row of descending if()'s that are
not connected. I had the 'else's gone at first, and it worked just fine,
but I've been given to understand that style is as important as substance.
Not that I agree, but when in Rome....

It may be just me but I admit to hating this particular style.

Nope, not just you. I don't like it either, as it's too cramped for my
taste. I was often involved in text presentation, as are we all, and I
'got' that white space is very important visually. This style might please
the K&R purist who is used to seeing reams of such code, but "I aren't one of
those critters!" lol!!

In any case, I've done due diligence here, and the next post will be of code
as I like to see it. Perhaps you'll like it better as well...

Thanks for the review, Barry!

 

[Barry Schwarz]

I see three in the function.

Ummm... yes, including the program name. I'm not sure how this is considered.
Launching the app itself (here, './wrap') invokes the first argument to the
code as argv[0], but is considered a launch with no arguments. Or is this
wrong. I'm talking to the user here who needn't have a clue about the code
itself.

Oh, you meant the comment to refer to the program. I thought it
referred to the function that immediately follows.

filename and line length.
*/

void wordwrap(FILE *ifp, FILE *ofp, char *wl)
{
char buf[MAX];
int c, i, space, count, length;
length = atoi(wl);
for (i = 0; i < MAX && ((c=getc(ifp)) != EOF); ++i)
buf = (char)c;

This cast serves no practical purpose but may be used to suppress an
annoying and irrelevant diagnostic from the compiler.

An annoying diagnostic from lclint, to be precise. Compiled and worked just
fine without it, but given the depth of criticism lurking hereabouts...

for (i = 0; buf != '\0'; ++i) {

When will buf be '\0'. The only time I can see is when the input
file contains a '\0'. Do you expect there to be only one at the end
of the file? I would expect most text files to not have any '\0' at
all. Maybe you want to add the following after the previous Input
loop:
buf = '\0';

Well, '\0' is the null character at the end of an string by definition, and
I guess I'm treating an ASCII text file as a string. In any case, I'm

Not a good plan. Strings are strings and files are files. The rules
are not the same for each.

reading a file into an array, and for the scale I'm treating at the moment,
it works. So the null character is the check point for EOF. However, that
won't be the general solution.

snip


<<Remove the del for email>>

 

[name]

<snip?

Not a good plan. Strings are strings and files are files. The rules
are not the same for each.

Yep. Which is why I need to understand this better. I guess the point is
that I need to learn how different files are put together so that 1) I can
check for file types 'wrap' shouldn't handle, and 2) treat what it should
handle correctly.

I'm going to study on this now and prepare for a second revision.

Thanks for your help, Barry. "Aaah'lll be BACK!!"

<grin>

 

[CBFalconer]

<snip?

I'm going to study on this now and prepare for a second revision.

Now that you have thrashed about this on your own, take a look at
the following. The function 'akeyboard()' is not portable, but
does what I want. Note that a -ve linelength parameter allows
ragged right (i.e. wrap) operation.

An exercise for you, if you wish, is to arrange to install the
extra justification blanks from the opposite end of the line on
alternate lines. This is intended to avoid 'rivers' of blanks in
the output.

/* ----- justify.c -----
Filter text file, right justifying by inserting
spaces between words. Words are anything separated
by blanks, tabs, newlines, formfeeds, bell, etc.

The single (optional) parameter is the output line
length, and defaults to 65. Execution without any
input redirections causes a help message.

This is a quick and dirty utility.
Released to public domain by:
<mailto:[email protected]>
*/

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>

#define RHDEFAULT 65
#define RHMIN 20

static int rhcol; /* right hand column limit */
static int ragged; /* No rh justification, 0 init */

/* ------------------- */

/* This is very likely to be non-portable */
/* DOES NOT check fp open for reading */
/* NULL fp is considered a keyboard here! */
static int akeyboard(FILE *fp)
{
#ifndef __TURBOC__
# ifdef __STDC__
/* This dirty operation allows gcc -ansi -pedantic */
extern int fileno(FILE *fp);
extern int isatty(int fn);
# endif
#endif
return ((fp != NULL) && isatty(fileno(fp)));
} /* akeyboard */

/* ------------------- */

static void help(char *phrase1, char *phrase2)
{
if (phrase1) fprintf(stderr, "%s", phrase1);
if (phrase2) fprintf(stderr, "%s", phrase2);
fprintf(stderr, "\n"
"Usage: justify [rightmargin] <infile >outfile\n"
" The default rightmargin is 65\n"
" and values less than 20 are rejected\n"
"\n"
"A large value of rightmargin will effectively\n"
"convert all paragraphs into single lines\n"
"\n"
"A negative rightmargin causes ragged right\n"
"\n"
"A blank line delimits paragraphs\n");
} /* help */

/* ------------------- */

static int initialize(int argc, char *argv[])
{
long rightcol;
char *err;

if (akeyboard(stdin) || (argc > 2)) {
help(NULL, NULL);
return 0;
}
rhcol = RHDEFAULT;
if (2 == argc) {
rightcol = strtol(argv[1], &err, 10);
if (rightcol < 0) {
rightcol = -rightcol;
ragged = 1;
}
if ((err == argv[1]) || (rightcol < RHMIN)) {
help("Bad argument: ", argv[1]);
return 0;
}
else rhcol = rightcol;
}
return 1;
} /* initialize */

/* ------------------- */

static void cleanup(void)
{
} /* cleanup */

/* ------------------- */

/* ================================== */
/* Routines for text input and output */
/* ================================== */

static void skipblanks(FILE *f)
{
int ch;

while ( (' ' == (ch = getc(f))) || ('\t' == ch) ||
('\v' == ch) || ('\f' == ch) || ('\a' == ch) )
continue;
ungetc(ch, f);
} /* skipblanks */

/* ------------------- */

/* The file is assumed to hold no control chars */
/* other than \n \t \v \a and \f. A blank line */
/* marks a paragraph ending word */
static int nextword(FILE *f, char *buffer, int max)
{
int i, ch;

skipblanks(f);
if (EOF == (ch = getc(f))) return 0;

/* Detect paragraph endings as \n\n */
if ('\n' == ch) {
skipblanks(f); ch = getc(f);
if ('\n' == ch) { /* paragraph ending */
buffer[0] = buffer[1] = ch; /* wd = "\n\n" */
buffer[2] = '\0';
/* now we have to absorb any more blank lines */
do {
skipblanks(f); ch = getc(f);
} while ('\n' == ch);
ungetc(ch, f);
return 1;
}
}
/* now ch holds the first non-blank. Use all printable */
if (EOF == ch) return 0;
if (!isgraph(ch)) {
fprintf(stderr, "'%c', 0x%x WARN: Invalid character\n",
ch, (unsigned)ch);
}

i = 0;
do {
buffer[i++] = ch;
if (i >= max) { /* truncate over long words */
i--;
break; /* leaving ch for next word */
}
ch = getc(f);
} while (isgraph(ch));

ungetc(ch, f); /* save for next word, may be \n */
buffer = '\0'; /* terminate string */
return 1;
} /* nextword */

/* ------------------- */

static void justify(char *ln, int wdgaps, int xtra, FILE *out)
{
int insert, i;
static int oddln = 0; /* for rt left blank insertion */
char ch;

#ifdef DEBUG
fprintf(out, "%2d %2d ", wdgaps, xtra);
#endif
insert = 0; oddln = !oddln;
if (wdgaps)
while (xtra > wdgaps) {
insert++; xtra -= wdgaps;
}
while ((ch = *ln++)) {
putc(ch, out);
if (' ' == ch) {
if (xtra) {
xtra--;
putc(' ', out);
}
for (i = insert; i; i--) putc(' ', out);
}
}
putc('\n', out);
} /* justify */

/* ------------------- */

static int filter(FILE *in, FILE *out)
{
char *buf;
char *ln;
int wdcount, lnlgh, wdlgh;
char *eop = "\n\n"; /* end of paragraph */
int done, endpar;

if (!(buf = malloc(rhcol+1))) exit(EXIT_FAILURE);
if (!(ln = malloc(rhcol+1))) exit(EXIT_FAILURE);

done = !nextword(in, buf, rhcol + 1);
endpar = !strcmp(buf, eop);

while (!endpar && !done) {
/* form paragraph */
wdlgh = strlen(buf);
wdcount = 0;
*ln = '\0'; lnlgh = 0;

while ((((lnlgh + wdlgh) < rhcol) || !lnlgh)
&& !done && !endpar) {
/* form a line */
if (lnlgh) ln[lnlgh++] = ' ';
strcpy(ln + lnlgh, buf);
lnlgh += wdlgh;
wdcount++;

done = !nextword(in, buf, rhcol + 1);
endpar = !strcmp(buf, eop);
wdlgh = strlen(buf);
}

/* dump the line, wdcount words */
if (endpar || done) lnlgh = rhcol;
if (ragged) fprintf(out, "%s\n", ln);
else justify(ln, wdcount-1, rhcol-lnlgh, out);

if (endpar) {
fputc('\n', out);
done = !nextword(in, buf, rhcol + 1);
endpar = !strcmp(buf, eop);
}
}
return 0;
} /* filter */

/* ------------------- */

int main(int argc, char *argv[])
{
if (!initialize(argc, argv)) return EXIT_FAILURE;
else {
(void)filter(stdin, stdout);
cleanup();
}
return 0;
} /* main */