Handle bad credentials

[elijahr]

Also:

• Fixes a bug where XXNetworkErrorXX wouldn’t be caught due to value of $? changing after rm call.
• Script has been run through shfmt.

[ekalinin]

Thanks for the PR. A few things need to be fixed before it can be merged:

1. Conflicts with master. The PR is based on 0.10.0. The $? fix is already on master as curl_status (#172), and the whole-file shfmt reformat touches most of the functions changed since then (--depth, --numbered, lint fixes). Please rebase onto current master and keep only the bad-credentials change, without the reformat.

2. --insert wipes the TOC on GNU sed. The shfmt commit changed the probe to if ! sed --version || true >/dev/null 2>&1, which is always true, so GNU sed also gets sed -i "". On Linux the old TOC is deleted, the new one is not inserted, and the tool still prints !! TOC was added. With --no-backup the backup is removed too. The existing tests don't catch it because they don't check the file after --insert.

3. False "bad credentials" error. awk '/Bad credentials/' runs over the rendered HTML of the user's document, so any markdown that mentions "Bad credentials" fails with exit 1 even with a valid token. Checking the HTTP status (401) instead of the response body would avoid this.

4. No tests. Please add a test for the bad-credentials case, e.g. GH_TOC_TOKEN=invalid gh-md-toc README.md expecting exit 1 and the error message.

[elijahr]

Thanks for the review. I've pushed changes for all four points:

1. Conflicts with master: I reverted the shfmt commit and merged current master in. My $? fix is gone since fix(md2html): check curl exit status for network errors #172 already covers it as curl_status, so the --depth, --numbered and lint changes are untouched. Compared with master, the PR now changes gh-md-toc, tests/tests.bats and one new test fixture.
2. GNU sed bug: fixed by the revert. The original sed --version check is back, so Linux no longer gets sed -i "".
3. False credential errors: bad credentials are now detected by HTTP status. curl appends the status code with -w '\n%{http_code}', and a 401 produces the bad-credentials error. The response body isn't checked for "Bad credentials" anymore. "API rate limit exceeded" had the same false positive, so it's now only treated as a rate limit on a non-200 response. Any other non-200 response exits 1 with GitHub API request failed (HTTP <code>) instead of quietly producing an empty TOC.
4. Tests: I added two:
   • GH_TOC_TOKEN=invalid should exit 1 with the bad-credentials error.
   • A fixture with headings "Bad credentials" and "API rate limit exceeded" should still produce a normal TOC.

The bad-credentials message now says which token source was used (GH_TOC_TOKEN or token.txt). The rate-limit message is the same as on master.

[ekalinin]

Thanks for the update. All four points from the previous review are fixed. One thing is still missing before this can be merged:

1. stdin input still fails silently. When reading from stdin (gh-md-toc -, line 447), the output of gh_toc_md2html goes straight to gh_toc_grab, so the new error markers are never checked there. printf '# A\n' | GH_TOC_TOKEN=bad ./gh-md-toc - still prints an empty TOC and exits 0, while the same input from a file now exits 1 with the bad-credentials error. Please handle XXBadCredentialsXX and XXApiErrorXX for stdin too, and add a test for it.

[elijahr]

Thanks! Fixed in 7ce7e0d.

The error checks are now in one function, gh_toc_check_api_error, which the file and stdin paths both call. gh-md-toc - now exits 1 with the same messages for bad credentials, other API errors, rate limits and network failures, instead of printing an empty TOC.

I added two stdin tests: one with an invalid token and one with no network access.

[ekalinin]

Thanks!