Skip to content

Comments

Some fixes in Diff command.#97

Merged
rhuijben merged 6 commits intoAmpScm:mainfrom
rinrab:main
Apr 17, 2025
Merged

Some fixes in Diff command.#97
rhuijben merged 6 commits intoAmpScm:mainfrom
rinrab:main

Conversation

@rinrab
Copy link
Contributor

@rinrab rinrab commented Apr 20, 2024

This pull-request does little fixes in the Diff command.

  • Use newer Subversion API possibilities to do diff in one thread. The SvnStreamWrapper is already used in the Write command, while diff was used the AprStreamFile.
  • Use Null Stream (System::IO::Stream::Null) instead of creating new MemoryStream if no ErrorSteam are set. This is needed to prevent creation a new MemoryStream on every diff invocation.

rinrab added 3 commits April 20, 2024 15:21
Use SvnStreamWrapper instead of AprStreamFile to implement this.
* Use Null Stream (System::IO::Stream::Null) instead of creating new MemoryStream if no ErrorSteam are set.
@rhuijben
Copy link
Member

Subversions diff can be rerouted to an external diff command. This command only works properly if there is really a file handle behind the stream. (Subversions obtains the backing handle via a private Api in this case)

@rinrab
Copy link
Contributor Author

rinrab commented Apr 20, 2024

Does SharpSvn support an external diff command? Because, as I found, the diff cmd property resets in the SvnClientContext::ApplyUserDiffConfig() function.

Could I solve the problem by adding check of diff_cmd for null and use different streams based on this?

@rhuijben
Copy link
Member

Sure, if you check for the safe case then we can go this route.

rinrab added 2 commits April 21, 2024 14:46
Create an instance of AprStreamFile if the diff_cmd is specified, or
SvnStreamWrapper if diff_cmd is null. The external diff tool (specified
via diff_cmd) supports only file streams, while libsvn_diff supports
both (file stream and SvnStream).

* Diff.cpp
  (SvnClient::Diff): Add a check of diff_cmd for null.
  (SvnClient::Diff): Move the body of the diff command to the SvnClient::InternalDiff() function.
  (SvnClient::InternalDiff): Move the body of the diff command here.
* SvnClient.h
  (InternalDiff): Add a declaration of the InternalDiff() function.
* src/SharpSvn/Commands/Diff.cpp
  * Revert change in code format which is unrelated; Replace
    `svn_error_t* r` with `svn_error_t *r` (space position changed).
@rinrab
Copy link
Contributor Author

rinrab commented Apr 22, 2024

I think I am done with implementation of the check for diff_cmd. Could you please review these changes?

Thanks!

@rinrab
Copy link
Contributor Author

rinrab commented May 10, 2024

@rhuijben Did you see my new commits?

@rhuijben rhuijben merged commit 7062eeb into AmpScm:main Apr 17, 2025
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.

2 participants