Skip to content
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

User Guide: scanner examples could be improved #4468

Closed
mwichmann opened this issue Jan 31, 2024 · 4 comments · Fixed by #4470
Closed

User Guide: scanner examples could be improved #4468

mwichmann opened this issue Jan 31, 2024 · 4 comments · Fixed by #4470

Comments

@mwichmann
Copy link
Collaborator

mwichmann commented Jan 31, 2024

Initiated from a scons-users thread, which may be found at https://pairlist4.pair.net/pipermail/scons-users/2024-January/009286.html

The examples in the User Guide chapter on scanners have some errors. The specific errors that are just incorrect code are in https://scons.org/doc/4.6.0/HTML/scons-user.html#id1490 (using a version-specific link intentionally): a missing closing paren; and a scanner function declaration that expects an arg parameter, but then the Scanner is not created with an argument kwarg, so the scanner won't actually be called with arg, and thus would produce:

TypeError: kfile_scan() missing 1 required positional argument: 'arg'

The examples in this chapter are not set to generate results via the usual bin/docs-create-example-outputs.py script and be included in the generated text, so we don't have an automated way to detect syntax errors.

There are also some inconsistencies... there's mention of a "foo" scanner (perhaps following on from the Builder chapter's examples of building from .foo sources), but the suffix used is actually .k. A fair bit of useful explanation seems to be missing, and if you did make the example in section 20.3 run, it wouldn't show anything interesting. The use of a path variable and a scanner path function is never actually elaborated on - it shows how to pass the path_function on SCanner creation, but the scanner function does not show making any use of the path tuple it will be called with as a result.

@mwichmann
Copy link
Collaborator Author

One more from the list thread:

env = Environment(ENV={'PATH': '/usr/local/bin'})

is used - that's poor practice, as it wipes out any default values of env['ENV'] and makes any actual execution of an external command unlikely to work well. Perhaps can be made to work for the example, but should introduce things in a better way, so the learning translates to "real life" usage scenarios.

@bdbaddog
Copy link
Contributor

bdbaddog commented Feb 1, 2024

One more from the list thread:

env = Environment(ENV={'PATH': '/usr/local/bin'})

is used - that's poor practice, as it wipes out any default values of env['ENV'] and makes any actual execution of an external command unlikely to work well. Perhaps can be made to work for the example, but should introduce things in a better way, so the learning translates to "real life" usage scenarios.

There are real world build environments where you don't want to use or reference anything from /usr/bin for example. So this can be a reasonable thing to do, but perhaps just needs more explanation?

@bdbaddog
Copy link
Contributor

bdbaddog commented Feb 1, 2024

In the 20.3 example the skeys perhaps should be ['.input'] ?
It's not really clear that we're registering a scanner to scan .k files with a builder who's input is .input files?

@mwichmann
Copy link
Collaborator Author

In the 20.3 example the skeys perhaps should be ['.input'] ?
It's not really clear that we're registering a scanner to scan .k files with a builder who's input is .input files?

yes, this was part of my questioning, a builder that makes .k files rather than turns them into something else was unexpected to me, as was scanning the .input rather than the .k. But it does "work" in the sense you get in the tree foo.k depending on foo.input plus the file included.

mwichmann added a commit to mwichmann/scons that referenced this issue Feb 1, 2024
Examples (except for the ones which are intentionally fragments)
can now be run, and they are set to do so via the <scons_example>
markup.  More explanation added. Syntax errors corrected.

Also reworded and expanded on some parts of the Scanner Objects
section of the manpage.

Fixes SCons#4468

Signed-off-by: Mats Wichmann <mats@linux.com>
mwichmann added a commit to mwichmann/scons that referenced this issue Feb 1, 2024
Examples (except for the ones which are intentionally fragments)
can now be run, and they are set to do so via the <scons_example>
markup.  More explanation added. Syntax errors corrected.

Also reworded and expanded on some parts of the Scanner Objects
section of the manpage.

Fixes SCons#4468

Signed-off-by: Mats Wichmann <mats@linux.com>
mwichmann added a commit to mwichmann/scons that referenced this issue Feb 1, 2024
Examples (except for the ones which are intentionally fragments)
can now be run, and they are set to do so via the <scons_example>
markup.  More explanation added. Syntax errors corrected.

Also reworded and expanded on some parts of the Scanner Objects
section of the manpage.

Fixes SCons#4468

Signed-off-by: Mats Wichmann <mats@linux.com>
mwichmann added a commit to mwichmann/scons that referenced this issue Feb 1, 2024
Examples (except for the ones which are intentionally fragments)
can now be run, and they are set to do so via the <scons_example>
markup.  More explanation added. Syntax errors corrected.

Also reworded and expanded on some parts of the Scanner Objects
section of the manpage.

Fixes SCons#4468

Signed-off-by: Mats Wichmann <mats@linux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants