Skip to content

Add leaveOpen overloads for SepReaderOptions.From* via SepTextReaderDisposers#530

Merged
nietras merged 13 commits into
mainfrom
copilot/add-leaveopen-flag-sepreadoptions
May 3, 2026
Merged

Add leaveOpen overloads for SepReaderOptions.From* via SepTextReaderDisposers#530
nietras merged 13 commits into
mainfrom
copilot/add-leaveopen-flag-sepreadoptions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 27, 2026

SepReader IO extension methods always closed the underlying stream/reader on dispose, unlike the corresponding writer APIs. This made ownership hard to control when composing readers over externally managed streams.

  • API surface

    • Add leaveOpen overloads for reader creation from:
      • Stream
      • Func<string, Stream>
      • TextReader
      • Func<string, TextReader>
    • Mirror the existing writer-style ownership pattern in both sync and async reader extensions.
  • Implementation

    • Use StreamReader(Stream, Encoding, Boolean, Int32, Boolean) for stream-backed reader overloads.
    • Restore reader-side disposal plumbing so SepReader leave-open behavior matches writer closely for both sync and async disposal paths.
    • Preserve existing default behavior (leaveOpen: false) for current callers.
  • Coverage

    • Add focused tests covering disposal ownership for:
      • stream-backed readers
      • text-reader-backed readers
    • Verify both leaveOpen: true and leaveOpen: false paths.
using var stream = GetCsvStream();
using var textReader = GetCsvTextReader();

// Existing behavior remains the default
using var reader1 = Sep.Reader().From(stream);
using var reader2 = Sep.Reader().From(textReader);

// New overloads keep the underlying source open
using var reader3 = Sep.Reader().From(stream, leaveOpen: true);
using var reader4 = Sep.Reader().From(textReader, leaveOpen: true);

> [!WARNING]
>
>

Agent-Logs-Url: https://github.com/nietras/Sep/sessions/e25d2005-3363-428e-86f5-1be461f8babc

Co-authored-by: nietras <10798831+nietras@users.noreply.github.com>
Copilot AI changed the title [WIP] Add LeaveOpen flag to SepReaderOptions for consistency with SepWriterOptions Add leave-open reader overloads for stream and text reader entry points Apr 27, 2026
Copilot AI requested a review from nietras April 27, 2026 18:54
@nietras
Copy link
Copy Markdown
Owner

nietras commented Apr 27, 2026

this is bad, you shouldn't add a new set of types for disposal

@nietras
Copy link
Copy Markdown
Owner

nietras commented Apr 27, 2026

also use StreamReader(Stream, Encoding, Boolean, Int32, Boolean) ctor

@nietras
Copy link
Copy Markdown
Owner

nietras commented Apr 27, 2026

@copilot continue work as detailed by previous comments, don't add unnecessary stuff, simplify, keep succinct.

Copilot AI and others added 2 commits April 27, 2026 19:15
Agent-Logs-Url: https://github.com/nietras/Sep/sessions/19904039-5cc6-4f0a-a6cd-55ebc5c309c7

Co-authored-by: nietras <10798831+nietras@users.noreply.github.com>
Agent-Logs-Url: https://github.com/nietras/Sep/sessions/19904039-5cc6-4f0a-a6cd-55ebc5c309c7

Co-authored-by: nietras <10798831+nietras@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 27, 2026

this is bad, you shouldn't add a new set of types for disposal

Updated in db7fc04. I removed the extra disposal types and restored the original SepReader disposal path.

also use StreamReader(Stream, Encoding, Boolean, Int32, Boolean) ctor

Updated in db7fc04. The stream-based reader overloads now use the StreamReader(..., leaveOpen) constructor directly.

@copilot continue work as detailed by previous comments, don't add unnecessary stuff, simplify, keep succinct.

Addressed in db7fc04. The change is simplified and limited to the native stream-based leaveOpen path.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • www.myget.org
    • Triggering command: /usr/bin/dotnet dotnet test /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj --nologo -c Debug --no-restore --filter FullyQualifiedName~SepReaderTest_Extensions_From --results-directory /tmp/sep-test-results-2 --logger trx;LogFileName=sep-reader-stream-leaveopen.trx (dns block)
    • Triggering command: /usr/bin/dotnet dotnet restore /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj --nologo --ignore-failed-sources (dns block)
    • Triggering command: /opt/hostedtoolcache/CodeQL/2.25.1/x64/codeql/csharp/tools/linux64/Semmle.Autobuild.CSharp /opt/hostedtoolcache/CodeQL/2.25.1/x64/codeql/csharp/tools/linux64/Semmle.Autobuild.CSharp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Add leave-open reader overloads for stream and text reader entry points Add leave-open reader overloads for stream entry points Apr 27, 2026
@nietras
Copy link
Copy Markdown
Owner

nietras commented Apr 29, 2026

@copilot ok you we're right to include Reader equivalent to below, so revert to that and ensure leaveOpen for Reader matches Writer closely.

interface ISepTextWriterDisposer
{
void Dispose(TextWriter writer);
ValueTask DisposeAsync(TextWriter writer);
}

sealed class SepTextWriterDisposer : ISepTextWriterDisposer
{
SepTextWriterDisposer() { }
public static SepTextWriterDisposer Instance { get; } = new();
public void Dispose(TextWriter writer) => writer.Dispose();
public ValueTask DisposeAsync(TextWriter writer) => writer.DisposeAsync();
}

sealed class NoopSepTextWriterDisposer : ISepTextWriterDisposer
{
NoopSepTextWriterDisposer() { }
public static NoopSepTextWriterDisposer Instance { get; } = new();
public void Dispose(TextWriter writer) { }
public ValueTask DisposeAsync(TextWriter writer) => ValueTask.CompletedTask;
}

@nietras
Copy link
Copy Markdown
Owner

nietras commented Apr 29, 2026

@copilot also see changes made in #123 for how to and what to call this pr

Copilot AI and others added 2 commits April 29, 2026 13:32
Agent-Logs-Url: https://github.com/nietras/Sep/sessions/f84c8c35-3a73-427d-87d8-8392b865b1ba

Co-authored-by: nietras <10798831+nietras@users.noreply.github.com>
Agent-Logs-Url: https://github.com/nietras/Sep/sessions/f84c8c35-3a73-427d-87d8-8392b865b1ba

Co-authored-by: nietras <10798831+nietras@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 29, 2026

@copilot ok you we're right to include Reader equivalent to below, so revert to that and ensure leaveOpen for Reader matches Writer closely.

interface ISepTextWriterDisposer
...

Updated in b9d8a63. I restored the reader-side disposer pattern, brought back the text-reader leave-open overloads/tests, and aligned async reader disposal with the same leave-open flow.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • www.myget.org
    • Triggering command: /usr/bin/dotnet dotnet test /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj --nologo -c Debug --no-restore --filter FullyQualifiedName~SepReaderTest_Extensions_From --results-directory /tmp/sep-test-results-pre --logger trx;LogFileName=sep-reader-prechange.trx (dns block)
    • Triggering command: /usr/bin/dotnet dotnet restore /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj --nologo --ignore-failed-sources (dns block)
    • Triggering command: /usr/bin/dotnet dotnet test /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj --nologo -c Debug --no-restore --filter FullyQualifiedName~SepReaderTest_Extensions_From --results-directory /tmp/sep-test-results-post --logger trx;LogFileName=sep-reader-postchange.trx (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Add leave-open reader overloads for stream entry points Add leave-open reader overloads for stream and text reader entry points Apr 29, 2026
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 29, 2026

@copilot also see changes made in #123 for how to and what to call this pr

Reviewed #123. This should follow that naming/style as the reader counterpart, e.g. Add leaveOpen overloads for SepReaderOptions.From via dispose action to ctor; the implementation already matches that in b9d8a63.

@nietras nietras changed the title Add leave-open reader overloads for stream and text reader entry points Add leaveOpen overloads for SepReaderOptions.From* via SepTextReaderDisposers May 3, 2026
@nietras nietras marked this pull request as ready for review May 3, 2026 12:29
@nietras
Copy link
Copy Markdown
Owner

nietras commented May 3, 2026

@copilot review PR after I fixed issues.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.58%. Comparing base (61cbfdb) to head (260c6c9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #530      +/-   ##
==========================================
- Coverage   99.64%   99.58%   -0.07%     
==========================================
  Files          56       57       +1     
  Lines        4794     4815      +21     
  Branches      538      540       +2     
==========================================
+ Hits         4777     4795      +18     
  Misses         14       14              
- Partials        3        6       +3     
Flag Coverage Δ
Debug 99.27% <100.00%> (+<0.01%) ⬆️
Release 99.61% <100.00%> (-0.18%) ⬇️
macos-latest 93.93% <100.00%> (+0.02%) ⬆️
ubuntu-24.04-arm 93.93% <100.00%> (+0.02%) ⬆️
ubuntu-latest 92.64% <100.00%> (-0.10%) ⬇️
windows-11-arm 93.93% <100.00%> (+0.02%) ⬆️
windows-latest 92.64% <100.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 3, 2026

@copilot review PR after I fixed issues.

Reviewed current head e01cb76. I don’t see any remaining issues in the leave-open reader changes, and the focused LeaveOpen tests pass on net10.0.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • www.myget.org
    • Triggering command: /tmp/dotnet-10.0.203/dotnet /tmp/dotnet-10.0.203/dotnet test /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj --filter SepReaderTest_Extensions_FromStream_LeaveOpen|SepReaderTest_Extensions_FromTextReader_LeaveOpen --no-restore (dns block)
    • Triggering command: /tmp/dotnet-10.0.203/dotnet /tmp/dotnet-10.0.203/dotnet restore /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj (dns block)
    • Triggering command: /tmp/dotnet-10.0.203/dotnet /tmp/dotnet-10.0.203/dotnet restore /home/REDACTED/work/Sep/Sep/src/Sep.Test/Sep.Test.csproj -p:RestoreIgnoreFailedSources=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from nietras May 3, 2026 12:43
Copilot finished work on behalf of nietras May 3, 2026 12:43
@nietras
Copy link
Copy Markdown
Owner

nietras commented May 3, 2026

Fix #529

@nietras nietras merged commit 650d02e into main May 3, 2026
39 checks passed
@nietras nietras deleted the copilot/add-leaveopen-flag-sepreadoptions branch May 3, 2026 12:50
github-actions Bot pushed a commit to Stella-sea/ryujinx-admin that referenced this pull request May 15, 2026
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [Sep](https://github.com/nietras/Sep) | `0.13.0` → `0.14.1` | ![age](https://developer.mend.io/api/mc/badges/age/nuget/Sep/0.14.1?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Sep/0.13.0/0.14.1?slim=true) |

---

### Release Notes

<details>
<summary>nietras/Sep (Sep)</summary>

### [`v0.14.1`](https://github.com/nietras/Sep/releases/tag/v0.14.1): 0.14.1

#### What's Changed

- Improve SepReaderOptions.Unescape/.Trim comments by [@&#8203;nietras](https://github.com/nietras) in [#&#8203;542](nietras/Sep#542)
- Improve SepReader/WriterExtensions.Strict() comments by [@&#8203;nietras](https://github.com/nietras) in [#&#8203;543](nietras/Sep#543)

**Full Changelog**: <nietras/Sep@v0.14.0...v0.14.1>

### [`v0.14.0`](https://github.com/nietras/Sep/releases/tag/v0.14.0): 0.14.0

#### What's Changed

- Add `leaveOpen` overloads for SepReaderOptions.From\* via SepTextReaderDisposers by [@&#8203;Copilot](https://github.com/Copilot) in [#&#8203;530](nietras/Sep#530)
- Bump MSTest from 4.2.1 to 4.2.2 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;533](nietras/Sep#533)
- Bump github/codeql-action from 4.35.2 to 4.35.3 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;532](nietras/Sep#532)
- Bump step-security/harden-runner from 2.19.0 to 2.19.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;531](nietras/Sep#531)
- Confirm statuses:write is the correct minimum permission for super-linter (not checks:write) by [@&#8203;Copilot](https://github.com/Copilot) in [#&#8203;540](nietras/Sep#540)

**Full Changelog**: <nietras/Sep@v0.13.0...v0.14.0>

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNzguMCIsInVwZGF0ZWRJblZlciI6IjQzLjE3OC4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Reviewed-on: https://git.ryujinx.app/projects/Ryubing/pulls/98
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No "LeaveOpen" flag on the SepReaderOptions, inconsistent with the SepWriterOptions.

2 participants