gitextensions icon indicating copy to clipboard operation
gitextensions copied to clipboard

Add conditionals to script parameters

Open SlugFiller opened this issue 1 year ago • 9 comments

Proposed changes

  • Add conditional parameters, e.g. {if:SelectedFiles} --files {SelectedFiles}{/if}. The condition is based on whether the parameter is available, e.g. the script was executed from the diff view and any files are selected. Otherwise, the text in between is discarded. Also added {ifnot:option} for the opposite condition/fallback.

Before

No way to condition parameters based on presence or absence.

After

Can use {if:parameterName} to determine of an option is present.

Test methodology

TBD

Test environment(s)

  • Git Extensions 33.33.33
  • Build 43e20df903d2826bb4f41e890830fac9f39c2537
  • Git 2.42.0.windows.1
  • Microsoft Windows NT 10.0.19045.0
  • .NET 6.0.24
  • DPI 96dpi (no scaling)
  • Portable: False
  • Microsoft.WindowsDesktop.App Versions
    Microsoft.WindowsDesktop.App 6.0.21 [D:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
    Microsoft.WindowsDesktop.App 6.0.24 [D:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


:black_nib: I contribute this code under The Developer Certificate of Origin.

SlugFiller avatar Nov 08 '23 23:11 SlugFiller

Currently leaving as draft because:

  1. It does not yet account for options that might be provided by a IScriptOptionsProvider but are not. Given that context sensitivity is the main point, having conditions based on the scriptOptionsProvider == null is necessary.
  2. Requires designing and adding tests.

SlugFiller avatar Nov 08 '23 23:11 SlugFiller

(Basically, the replacement of script options works kind of "inverse": If there is a typo or there is no valid replacement, the option is not replaced but provided as is - without any error message.)

This PR will enable users to handle the cases "no blobs (files) exist / are selected" and "no visible control with blobs (files)".

@SlugFiller, are you going to follow up?

mstv avatar May 12 '24 22:05 mstv

Ah, I kind of forgot about this in the long wait for the file scripts. And was busy with other stuff. Will try to get back into this soon. And maybe work on the other thing I was planning, which is scripts for remote branches in the left sidebar (currently, they are only available for local branches, and they only isolate by commit, not by exact branch)

SlugFiller avatar May 13 '24 04:05 SlugFiller

I've noticed that SelectedRelativePaths is available when running the script from the revision list. Not only is this making this confusing/difficult to test, it makes me wonder if this PR is even still relevant. i.e. Under what condition would an option "not be available"?

I think #11734 inadvertently supersedes this one. At least for any use case I had in mind.

If no one has an alternative use case, I may as well close this.

SlugFiller avatar Aug 03 '24 15:08 SlugFiller

This PR does still make much sense because there can be no selected file, e.g. for empty commits and by means of Ctrl+Click.

mstv avatar Aug 03 '24 15:08 mstv

it makes me wonder if this PR is even still relevant. i.e. Under what condition would an option "not be available"?

You are right. The scripts can check on their own whether an option has a value or not, e.g. like:

cmd /c if "{SelectedRelativePaths}" == "" ( code . ) else ( code . --goto {{SelectedRelativePaths}}:{LineNumber} )

mstv avatar Aug 29 '24 16:08 mstv

It's not really what I meant. The way this PR is originally designed, is that it allows the if to operate when the parameter is null. Normally, it would either not replace (leaving {parameterName} as a string inside the command), or throw an exception.

However, from what I can tell, the current changes make it so that the parameter is always present. It is simply sometimes an empty string. I probably should do more experimentation to verify this.

If this is the case, one avenue I can chase is to have the if recognize an empty result/string as being a "false", and not just null. I could also try improving it so that unknown parameters are also interpreted as "false" (currently, it doesn't do a replacement at all). This could be used as a degree of forward compatibility. I'll have to experiment and see if any of it leads to anything useful.

SlugFiller avatar Aug 29 '24 22:08 SlugFiller