Implement client-side pre- and post-commit hooks

[RLThomaz]

trafficstars

Implement client-side pre- and post-commit hooks JohnstonCode/svn-scm#1355

I've tested it locally and it works using VSCode commands, but did not work directly on the SCM - is it supposed to in debug mode?

[JohnstonCode]

Do you think there would be more benefit from setting the hooks in the config to be an array of string path so multiple hooks could be added for each hook?

[JohnstonCode]

Your current implementation will only work if you run the commit command. I'm not 100% sure where the best place would be to put the hook check code.

[RLThomaz]

Do you think there would be more benefit from setting the hooks in the config to be an array of string path so multiple hooks could be added for each hook?

Sure, I cannot think of any reason against it. Actually, I believe this would be of great benefit for multi-root workspaces. I'm thinking of sending the repository name as an input argument to the hooks. This allows the user to control if a hook should do anything for a given repository. SVN does something similar but includes the transaction id - which we won't have on the client side.

Your current implementation will only work if you run the commit command. I'm not 100% sure where the best place would be to put the hook check code.

~~Yeah, that's what I was talking about in the PR comment. I'm not - at all - familiar with VSCode extensions, I understand that you can register commands - triggered manually using the command palette Ctrl + Shift + P - and that's the current behaviour. However, how does the SCM invoke this extension? Shouldn't it just call the command for us? I've noticed that using the SCM still triggers the extension, so which file is it actually calling?~~

Ok, I just rebuilt the extension and it is working using the SCM UI. So I guess your question is regarding if we should have the hook available somewhere else, correct?

If so, "will only work if you run the commit command" is the intended behaviour. I believe the hooks should all be there if they are, well, commit-hooks - such as pre-commit or post-commit. Of course, we could add extra hooks - SVN implements the following server-side hooks:

start-commit
pre-commit (added by this PR)
post-commit (added by this PR)
pre-revprop-change
post-revprop-change
pre-lock
post-lock
pre-unlock
post-unlock

[RLThomaz]

@JohnstonCode, I've just committed an improved version. Now the settings allow an array of objects, each object contains an array/string of commands. In addition, each object may contain a project name and branch. That way the hooks are only executed for the corresponding repository/project/branch. If project/branch is omitted the hooks will execute for all/any options available.

I've noticed I'm not really solving the problem in #1355, they want to be able to change the actual commit-transaction body. I think that's possible, but would require very specific hooks regarding the scope of what they do - in this case, the command would have to return back the updated transaction body. What do you think?

[JohnstonCode]

I think the hooks would be better placed here https://github.com/JohnstonCode/svn-scm/blob/7b45ad36bb21ebb71f72e140664b12cdf8b6f329/src/repository.ts#L1056 As your current hook only run on the single commit commands and not the commitWithMessage command

[RLThomaz]

I think the hooks would be better placed here

https://github.com/JohnstonCode/svn-scm/blob/7b45ad36bb21ebb71f72e140664b12cdf8b6f329/src/repository.ts#L1056

Interesting, so there is an enumeration specifying which command is being executed, correct? Ok, I can totally see the benefits of running the hooks there, but it would be just a call to a public function of Hook passing as arguments the operation (Operation) and this (Repository). We would need two functions in Hook, a pre-operation call and a post-operation call. This way those functions would be responsible for figuring out which hooks should be called given the operation type.

As your current hook only run on the single commit commands and not the commitWithMessage command

By the way, it's there on commitWithMessage too.

[RLThomaz]

Alright, just pushed a more generic implementation of Hook.

• I followed your suggestion and moved the execution to https://github.com/JohnstonCode/svn-scm/blob/7b45ad36bb21ebb71f72e140664b12cdf8b6f329/src/repository.ts#L1056
• Hooks now hold several properties.
  • Disposition: pre or post-command execution.
  • Command Type: System call or VS Code command.
  • Operation: SVN operation type on which the commands should be executed.

I believe the addition of VS Code command support should give us something similar to @rroman6174's implementation. The only thing that's lacking now is a solution to #1355. Perhaps we can use stdout from here to update whatever the command needs - for example, the commit message. What do you think, @JohnstonCode?

Here's one example of what we can do now:

{
  "svn.hooks": [
      {
        "commandDisposition": "Pre",
        "commandType": "VSCode",
        "commandOperation": "Commit",
        "command": "search.action.openNewEditor",
        "branch": "trunk",
        "project": "MyProject"
      },
      {
        "commandDisposition": "Pre",
        "commandType": "System",
        "commandOperation": "Commit",
        "command": "/bin/bash -c 'echo \"Pre-commit hook failed: \n this is an error.\" 1>&2; exit 1' ",
        "branch": "trunk",
        "project": "MyProject"
      }
    ]
}