ping
On 10/31/18 7:28 AM, Martijn van Duren wrote:
> On 8/15/18 8:58 PM, Ingo Schwarze wrote:
>> Hi Martijn,
>>
>> i agree with Todd that in implementing an option invented by GNU,
>> we should follow GNU semantics unless there are very strong reasons
>> to diverge. In this case, GNU -i semantics is maybe not the only
>> possibility that could be regarded as reasonable, but it certainly
>> isn't unreasonable: Execute the script independently for each file
>> in turn, but let the 'q' command still exit the editor completely,
>> rather than just moving to the next file.
>>
>> So, i agree with resetting the hold space when moving from one file
>> to the next in -i mode. But i think the variable HS ought to remain
>> in process.c and not be moved to extern.h and not be manipulated from
>> main.c.
>>
>> Instead, there already is a function to reset internal processor
>> state when moving to the next file with -i: resetranges().
>> Can you add the two lines to clear the hold space into that
>> function and resend the diff?
>>
>> If you want, you can make the name of the function more generic,
>> for example reset_processor().
>>
>> Also, in the manual page, while saying that the hold space is reset,
>> also mention what happens to line numbers and ranges, be even more
>> explicit that all this applies *only* for -i, and avoid the future
>> tense, for example like this:
>>
>> In
>> .Fl i
>> mode, line numbers and the hold space are reset between files,
>> and no range extends from one file into the next.
>>
>> Finally, in the description of the 'q' command, we can also be
>> more explicit:
>>
>> Branch to the end of the script and quit
>> .Nm
>> without starting a new cycle or a new file.
>>
>> Yours,
>> Ingo
>>
> I send the diff below to Ingo some time ago, but never got a reply.
> Maybe someone else is willing to look at it?
>
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/sed/extern.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 extern.h
> --- extern.h 1 Aug 2017 18:05:53 -0000 1.13
> +++ extern.h 23 Aug 2018 08:11:48 -0000
> @@ -53,8 +53,9 @@ __dead void error(int, const char *, ...
> void warning(const char *, ...);
> int mf_fgets(SPACE *, enum e_spflag);
> int lastline(void);
> +void finish_file(void);
> void process(void);
> -void resetranges(void);
> +void resetstate(void);
> char *strregerror(int, regex_t *);
> void *xmalloc(size_t);
> void *xreallocarray(void *, size_t, size_t);
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/sed/main.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 main.c
> --- main.c 11 Jul 2018 06:57:18 -0000 1.37
> +++ main.c 23 Aug 2018 08:11:48 -0000
> @@ -310,6 +310,30 @@ again:
> return (NULL);
> }
>
> +void
> +finish_file(void)
> +{
> + if (infile != NULL) {
> + fclose(infile);
> + if (*oldfname != '\0') {
> + if (rename(fname, oldfname) != 0) {
> + warning("rename()");
> + unlink(tmpfname);
> + exit(1);
> + }
> + *oldfname = '\0';
> + }
> + if (*tmpfname != '\0') {
> + if (outfile != NULL && outfile != stdout)
> + fclose(outfile);
> + outfile = NULL;
> + rename(tmpfname, fname);
> + *tmpfname = '\0';
> + }
> + outfname = NULL;
> + }
> +}
> +
> /*
> * Like fgets, but go through the list of files chaining them together.
> * Set len to the length of the line.
> @@ -347,25 +371,7 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
> sp->len = 0;
> return (0);
> }
> - if (infile != NULL) {
> - fclose(infile);
> - if (*oldfname != '\0') {
> - if (rename(fname, oldfname) != 0) {
> - warning("rename()");
> - unlink(tmpfname);
> - exit(1);
> - }
> - *oldfname = '\0';
> - }
> - if (*tmpfname != '\0') {
> - if (outfile != NULL && outfile != stdout)
> - fclose(outfile);
> - outfile = NULL;
> - rename(tmpfname, fname);
> - *tmpfname = '\0';
> - }
> - outfname = NULL;
> - }
> + finish_file();
> if (firstfile == 0)
> files = files->next;
> else
> @@ -405,7 +411,7 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
> fchmod(fileno(outfile), sb.st_mode & ALLPERMS);
> outfname = tmpfname;
> linenum = 0;
> - resetranges();
> + resetstate();
> } else {
> outfile = stdout;
> outfname = "stdout";
> Index: process.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/sed/process.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 process.c
> --- process.c 13 Dec 2017 16:06:34 -0000 1.33
> +++ process.c 23 Aug 2018 08:11:48 -0000
> @@ -196,6 +196,7 @@ redirect:
> if (!nflag && !pd)
> OUT();
> flush_appends();
> + finish_file();
> exit(0);
> case 'r':
> if (appendx >= appendnum) {
> @@ -312,9 +313,12 @@ applies(struct s_command *cp)
> * Reset all inrange markers.
> */
> void
> -resetranges(void)
> +resetstate(void)
> {
> struct s_command *cp;
> +
> + free(HS.back);
> + memset(&HS, 0, sizeof(HS));
>
> for (cp = prog; cp; cp = cp->code == '{' ? cp->u.c : cp->next)
> if (cp->a2)
> Index: sed.1
> ===================================================================
> RCS file: /cvs/src/usr.bin/sed/sed.1,v
> retrieving revision 1.56
> diff -u -p -r1.56 sed.1
> --- sed.1 11 Jul 2018 06:47:38 -0000 1.56
> +++ sed.1 23 Aug 2018 08:11:48 -0000
> @@ -106,6 +106,9 @@ It is not recommended to give a zero len
> .Ar extension
> when in place editing files, as it risks corruption or partial content
> in situations where disk space is exhausted, etc.
> +In
> +.Fl i
> +mode, the hold space, line numbers, and ranges are reset between files.
> .It Fl r
> An alias for
> .Fl E ,
> @@ -392,7 +395,7 @@ Write the pattern space to standard outp
> Write the pattern space, up to the first newline character,
> to the standard output.
> .It [1addr] Ns Ic q
> -Branch to the end of the script and quit without starting a new cycle.
> +Branch to the end of the script and quit without starting a new cycle or
> file.
> .It [1addr] Ns Ic r Ar file
> Copy the contents of
> .Ar file
>
