Closed musvaage closed 1 year ago
I'll do a more complete review shortly. Most of these are okay. I noticed one replacement that wasn't a typo (src/main.c) and a couple of replacements in email quotations (flex.texi). I'll get to a proper keyboard and mark them.
Looking at this, you've done some good work in spotting things that need fixed. But we're dealing with multiple kinds of texts. Some of them are documentation, some are program text, some are comments. This is a lot to review in one go. The usual way that git solves that problem is to submit multiple smaller pull requests. I'd guess that one pr per file is about the right balance. Are you fluent enough with git to do that?
wasn't a typo src/main.c
please clarify
all the changes are not in the code but in documentation and comments in code
Are you fluent enough with git to do that?
@westes
AFAIK, bulk fixes are preferred.
$ cat spelling
grep -n Changeed flex/NEWS
grep -n addding flex/src/ccl.c
grep -n ancestal flex/src/c99-flex.skl
grep -n anotating flex/examples/README
grep -n asssemble flex/tests/testmaker.sh
grep -n becaae flex/tests/ruleset.sh
grep -n cheacter flex/src/flexdef.h
grep -n complementery flex/tests/flexname.rules
grep -n connncerned flex/NEWS
grep -n declarationns flex/src/c99-flex.skl
grep -n enhabcing flex/tests/README
grep -n explictly flex/ONEWS
grep -n genertated flex/tests/README
grep -n genetated flex/doc/flex.texi
grep -n guisdance flex/doc/flex.texi
grep -n haooen flex/tests/yyless.rules
grep -n incrememnt flex/src/cpp-flex.skl
grep -n indepedent flex/tests/README
grep -n langage flex/src/c99-flex.skl
grep -n langage flex/doc/flex.texi
grep -n mamual flex/doc/flex.texi
grep -n natine flex/doc/flex.texi
grep -n neaningful flex/doc/flex.texi
grep -n orotocol flex/tests/README
grep -n outputing flex/src/misc.c
grep -n procede flex/autogen.sh
grep -n redefinining flex/doc/flex.texi
grep -n repoduce flex/examples/manual/j2t.lex
grep -n requred flex/src/c99-flex.skl
grep -n requred flex/src/go-flex.skl
grep -n rewreites flex/doc/flex.texi
grep -n rewute flex/src/c99-flex.skl
grep -n rewute flex/src/go-flex.skl
grep -n rtelying flex/tests/lineno.rules
grep -n seciton flex/examples/manual/j2t.lex
grep -n segfalt flex/NEWS
grep -n selrcted flex/src/flexdef.h
grep -n specfied flex/src/flexdef.h
grep -n specifing flex/src/main.c
grep -n statment flex/src/flexdef.h
grep -n stright flex/src/main.c
grep -n supercedes flex/ONEWS
grep -n thuas flex/doc/flex.texi
grep -n trigggered flex/NEWS
grep -n tweeks flex/NEWS
grep -n unduely flex/NEWS
grep -n unncomfortably flex/doc/flex.texi
grep -n usuually flex/doc/flex.texi
grep -n valuued flex/doc/flex.texi
grep -n whereever flex/src/tblcmp.c
grep -n whuch flex/doc/flex.texi
grep -n whrn flex/doc/flex.texi
$ . spelling
248:*** Changeed output formats from octal to hexadecimal
138: * addding each char in a that is not in b.
262:/* In the ancestal C back end this was a void pointer, meant to be
5: - debflex.awk, an awk script for anotating flex debug output.
3:# testmaker.sh - asssemble tests from backend-independent rulesets and
69:# posixlycorrect is a special case becaae we need to set POSIXLY_CORRECT
308: bool always_interactive;// always use cheacter-by-character input
27: * For the complementery test, see lexcompat.rules.
113: the build. This is useful when you're less connncerned about the
2059: /* open scope of user declarationns */
92:whole job can be done by enhabcing testmaker.m4.
990: writing an interactive scanner one must explictly call fflush()
100:m4 expansion doesn't leak through the genertated scanner.
4536:it might be a generic type, or no such field might be genetated at
8871:the existing tests should not be difficult. There is some guisdance
24: * Test yyless. What should haooen is:
4189: /* t32 is a plain int. copy data, then incrememnt p. */
99:The "quotes" test is also backend-indepedent; it's testing that
264: * wouldn't easily port to other langages.
1202:In target langages with automatic memory allocation and arrays, none
2550:Bison is also retargetable to langages other than C. Outside the
4371:respectively. This may not be true in other target langages,
8764:Otherwise, knowledge of each target langage's syntax lives in a
8864:produce code in your desired target langage.
9130:@c Remaining problem points for the multilangage interface
8795:single quote (ASCII 39) @footnote{See chapter 3.2 of the m4 mamual,
2484:whatever natine type is associated with I/O streams. It may be
9034:FLEX_SCANNER: Not neaningful outside of the C back end, and not defined.
90:If you can express your test as a ruleset within in this orotocol,
477:/* out - various flavors of outputing a (possibly formatted) string for the
25:# and procede with the "normal" build procedures.
9049:support redefinining this hook with #define. Instead, set it with
217: * repoduce @enumerate lists
224:#include <unistd.h> /* requred for isatty() */
156:#include <unistd.h> /* requred for isatty() */
2730:yystart(), yybegin(), yyunput(), yypanic() - and rewreites them adding a
971:%# yymore has a magic rewute rule. It's declared here, rather than with the other
928:%# yymore has a magic rewute rule. It's declared here, rather than with the other
33: * do magic rewrites rather than rtelying on the C preprocessor.
152:^"= "[A-Z]" ="\n"="* { /* we create a seciton for each category */
161:*** a segfalt involving yyrestart(NULL) has been fixed
1035:/* return the correct file suffix for the selrcted back end */
344: char *yydecl; // user-specfied prototype for yylex.
1297: /* If the user explicitly requested posix compatibility by specifing the
921:/* Generate a data statment for a two-dimensional array. */
1287: /* This has to be a stright textual substitution rather
932: - Note that this patch supercedes patch #1 for release 2.3,
3158:consequently everywhere; thuas there is no reason to use this option
608:** removed some warnings which some tests trigggered with the -s option
454:** slight tweeks to the flex_int*_t types
405: Makefiles were using it and it can be unduely confusing
9065:unncomfortably long and hard on the eyeballs. We leave
2758:@ref{Lex and Posix}. It will usuually be a no-op on back ends other
3141:This is a string-valuued option with which you can specify an expansion
492: * taken care of by whereever "deflink" points. "totaltrans" is the total
3142:of the yyterminate() hook. whuch normally causes the generated
2732:is set to false whrn you select or default to the C back end, and to
$
wasn't a typo src/main.c
please clarify
all the changes are not in the code but in documentation and comments in code
Sure. "transet" is [short] for "transition set." It's not a typo and "transit" would be a confusing replacement. Transet used to be a variable name. Not sure whether it still is so I need a minute to review HEAD and propose a fix to the docs if not.
You've done a good thing.
all the changes are not in the code but in documentation and comments in code
@Mightyjo
Can you confirm the accuracy of the above statement?
variable name
That deserves a separate ticket than the current one.
$ sed -n 1119,1121p flex-2.5.4/gen.c
/* Need to define the transet type as a size large
* enough to hold the biggest offset.
*/
$
Can you confirm the accuracy of the above statement?
If so, then I think this is ready to be merged.
@westes
AFAIK, bulk fixes are preferred.
@westes is our maintainer. Whatever he says is preferred.
variable name
That deserves a separate ticket than the current one.
"transet" isn't a variable name in HEAD, so that's not an issue.
I concur that this PR only touches documentation strings.
@musvaage, can you fork this repo and submit the same PR to your own fork? That will cause Github to run the regression tests, etc. against your fork to demonstrate that no functionality changed.
I don't have a setup for running tests.
only touches documentation strings
probably unnecessary to inform but all 256 tests passed
$ cd /tmp
$ git clone -q https://github.com/vaerksted/flex.git
$ cd flex && git log -1 | head -n 1
commit 276533632c043d3a9d6d49d567bb0e08c32f5aa0
$ ed -s src/flexdef.h <<<'47,55p'
#include <string.h>
#include <math.h>
#ifdef HAVE_ASSERT_H
#include <assert.h>
#else
#define assert(Pred)
#endif
$ sed -i "/math.h/a #include <malloc.h>" src/flexdef.h
$ ed -s src/flexdef.h <<<'47,55p'
#include <string.h>
#include <math.h>
#include <malloc.h>
#ifdef HAVE_ASSERT_H
#include <assert.h>
#else
#define assert(Pred)
#endif
$
cf: flex.SlackBuild (mirror) regarding the sed command
# Fix an issue introduced with recent glibc versions. Thanks to LFS:
sed -i "/math.h/a #include <malloc.h>" src/flexdef.h
then ran ./autogen.sh, ./configure, make, and make check
That sed isn't needed on master (see https://github.com/westes/flex/issues/436 & https://github.com/westes/flex/issues/499).
sed isn't needed
An e-mail was sent to Pat and the Slackware crew.
cause Github to run the regression tests
pipelines/workflows don't appear to run on forks
To be clear, the sed is needed with the last release, just not on master.
not on master
thanks for the clarification
hope the Slackware crew doesn't mind the extra incoming e-mail
$ date +"%d %b %Y"
19 Nov 2022
$ cd /tmp
$ rm -rf flex
$ git clone -q https://github.com/vaerksted/flex.git
$
without the sed command no issues noted with the build
running ./autogen.sh, ./configure, make, and make check
2 workflows awaiting approval
at this point of time possibly a maintainer could approve running the 2 workflows
https://github.com/vaerksted/flex.git
2 workflows awaiting approval
at this point of time possibly a maintainer could approve running the 2 workflows
I checked your vaerksted/flex fork. It's set up to run its own checks. You just have to make your changes in a branch (not in master) then issue a PR to master on vaerksted/flex. See my fork (mightyjo/flex) for examples. Once the checks pass on your local PR, you can issue it again against westes/flex and it'll have the same result (provided you're up to date with HEAD).
This way, you won't have to chase the maintainer build environment and everyone will be able to confirm your test results. (Plus you can test features from your phone. I love that.)
running the 2 workflows appears to have been approved
I don't have a setup for running tests.
Nice!
I'm still waiting for this to be merged.
Since you never split it up as requested, I'm having to find a larger block of time to review it. Which I Haven't been able to, yet.
NEWS ONEWS autogen.sh doc/flex.texi examples/README examples/manual/j2t.lex src/c99-flex.skl src/ccl.c src/cpp-flex.skl src/flexdef.h src/go-flex.skl src/main.c src/misc.c src/tblcmp.c tests/README tests/flexname.rules tests/lineno.rules tests/ruleset.sh tests/testmaker.sh tests/yyless.rules