Skip to content

Commit 4dae288

Browse files
lzybkrlzybkr
committed
ValidateAndAcceptLine improvements
Script block arguments are now only validated as arguments to specific cmdlets (which are in a user modifiable whitelist.) A custom validation handler must now throw an exception to report an error, returning an object is now ignored. A sample custom validation handler was added to demonstrate typo correction. Fixes #162
1 parent ce56092 commit 4dae288

File tree

6 files changed

+120
-67
lines changed

6 files changed

+120
-67
lines changed

‎PSReadLine/BasicEditing.cs

Lines changed: 48 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -258,73 +258,77 @@ private bool AcceptLineImpl(bool validate)
258258
return true;
259259
}
260260

261-
private string Validate(Ast rootAst)
261+
class CommandValidationVisitor : AstVisitor
262262
{
263-
if (_parseErrors != null && _parseErrors.Length > 0)
263+
private readonly Ast _rootAst;
264+
internal string detectedError;
265+
266+
internal CommandValidationVisitor(Ast rootAst)
264267
{
265-
// Move the cursor to the point of error
266-
_current = _parseErrors[0].Extent.EndOffset;
267-
return _parseErrors[0].Message;
268+
_rootAst = rootAst;
268269
}
269270

270-
var commandAsts = rootAst.FindAll(a => a is CommandAst, true);
271-
if (_engineIntrinsics != null)
271+
public override AstVisitAction VisitCommand(CommandAst commandAst)
272272
{
273-
foreach (var ast in commandAsts)
273+
var commandName = commandAst.GetCommandName();
274+
if (commandName != null)
274275
{
275-
var commandAst = (CommandAst)ast;
276-
var commandName = commandAst.GetCommandName();
277-
if (commandName != null)
276+
if (_singleton._engineIntrinsics != null)
278277
{
279-
var commandInfo = _engineIntrinsics.InvokeCommand.GetCommand(commandName, CommandTypes.All);
280-
if (commandInfo == null && !UnresolvedCommandCouldSucceed(commandName, rootAst))
278+
var commandInfo = _singleton._engineIntrinsics.InvokeCommand.GetCommand(commandName, CommandTypes.All);
279+
if (commandInfo == null && !_singleton.UnresolvedCommandCouldSucceed(commandName, _rootAst))
281280
{
282-
_current = commandAst.CommandElements[0].Extent.EndOffset;
283-
return string.Format(PSReadLineResources.CommandNotFoundError, commandName);
281+
_singleton._current = commandAst.CommandElements[0].Extent.EndOffset;
282+
detectedError = string.Format(PSReadLineResources.CommandNotFoundError, commandName);
283+
return AstVisitAction.StopVisit;
284284
}
285285
}
286-
}
287-
}
288286

289-
string errorMessage = null;
290-
if (_options.ValidationHandler != null)
291-
{
292-
try
293-
{
294-
dynamic validationResult = _options.ValidationHandler(rootAst.Extent.Text);
295-
if (validationResult != null)
287+
if (commandAst.CommandElements.Any(e => e is ScriptBlockExpressionAst))
296288
{
297-
errorMessage = validationResult as string;
298-
if (errorMessage == null)
299-
{
300-
try
301-
{
302-
errorMessage = validationResult.Message;
303-
_current = validationResult.Offset;
304-
}
305-
catch {}
306-
}
307-
if (errorMessage == null)
289+
if (_singleton._options.CommandsToValidateScriptBlockArguments == null ||
290+
!_singleton._options.CommandsToValidateScriptBlockArguments.Contains(commandName))
308291
{
309-
errorMessage = validationResult.ToString();
292+
return AstVisitAction.SkipChildren;
310293
}
311294
}
312295
}
313-
catch (Exception e)
296+
297+
if (_singleton._options.CommandValidationHandler != null)
314298
{
315-
errorMessage = e.Message;
316299
try
317300
{
318-
// If the exception object has an Offset property, we use it.
319-
// If not, this will throw an exception which we ignore.
320-
dynamic em = e;
321-
_current = em.Offset;
301+
_singleton._options.CommandValidationHandler(commandAst);
302+
}
303+
catch (Exception e)
304+
{
305+
detectedError = e.Message;
322306
}
323-
catch {}
324307
}
308+
309+
return !string.IsNullOrWhiteSpace(detectedError)
310+
? AstVisitAction.StopVisit
311+
: AstVisitAction.Continue;
312+
}
313+
}
314+
315+
private string Validate(Ast rootAst)
316+
{
317+
if (_parseErrors != null && _parseErrors.Length > 0)
318+
{
319+
// Move the cursor to the point of error
320+
_current = _parseErrors[0].Extent.EndOffset;
321+
return _parseErrors[0].Message;
322+
}
323+
324+
var validationVisitor = new CommandValidationVisitor(rootAst);
325+
rootAst.Visit(validationVisitor);
326+
if (!string.IsNullOrWhiteSpace(validationVisitor.detectedError))
327+
{
328+
return validationVisitor.detectedError;
325329
}
326330

327-
return errorMessage;
331+
return null;
328332
}
329333

330334
private bool UnresolvedCommandCouldSucceed(string commandName, Ast rootAst)

‎PSReadLine/Changes.txt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22

33
New features:
44
* Enter now does some extra validation before accepting the input. If there are any parse
5-
errors, command not found, or parameter binding errors, and error message is displayed
6-
and you can continue editing. Your history and the screen won't be cluttered with errors.
5+
errors or a command is not found, an error message is displayed, but you can continue editing,
6+
the erroneous line will not be added to history, and the error message will be cleared after
7+
you make an edit.
78
You can press Enter a second time to force accepting the line if you choose.
89

910
If you don't like the new behavior for Enter, you can revert to the old behavior with:
@@ -38,8 +39,9 @@ New options:
3839
-ErrorBackgroundColor
3940
Colors used when ValidateAndAcceptLine reports an error
4041

41-
-ValidationHandler
42+
-CommandValidationHandler
4243
A delegate that is called from ValidateAndAcceptLine that can perform custom validation
44+
and also fix the command line, e.g. to correct common typos.
4345

4446
New cmdlet:
4547
* Remove-PSReadlineKeyHandler

‎PSReadLine/Cmdlets.cs

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Collections.ObjectModel;
34
using System.Diagnostics.CodeAnalysis;
45
using System.Management.Automation;
6+
using System.Management.Automation.Language;
57
using System.Reflection;
68
using System.Linq;
79

@@ -131,7 +133,22 @@ public PSConsoleReadlineOptions(string hostName)
131133
HistorySaveStyle = DefaultHistorySaveStyle;
132134
HistorySavePath = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData)
133135
+ @"\PSReadline\" + hostName + "_history.txt";
134-
ValidationHandler = null;
136+
CommandValidationHandler = null;
137+
CommandsToValidateScriptBlockArguments = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
138+
{
139+
"ForEach-Object", "%",
140+
"Invoke-Command", "icm",
141+
"Measure-Command",
142+
"New-Module", "nmo",
143+
"Register-EngineEvent",
144+
"Register-ObjectEvent",
145+
"Register-WMIEvent",
146+
"Set-PSBreakpoint", "sbp",
147+
"Start-Job", "sajb",
148+
"Trace-Command", "trcm",
149+
"Use-Transaction",
150+
"Where-Object", "?", "where",
151+
};
135152
}
136153

137154
public EditMode EditMode { get; set; }
@@ -156,11 +173,23 @@ public PSConsoleReadlineOptions(string hostName)
156173
public Func<string, bool> AddToHistoryHandler { get; set; }
157174

158175
/// <summary>
159-
/// This handler is called from ValidateAndAcceptLine. If a non-null,
160-
/// non-empty string is returned, or if an exception is thrown,
161-
/// validation fails and the error is reported.
176+
/// This handler is called from ValidateAndAcceptLine.
177+
/// If an exception is thrown, validation fails and the error is reported.
178+
/// </summary>
179+
public Action<CommandAst> CommandValidationHandler { get; set; }
180+
181+
/// <summary>
182+
/// Most commands do not accept script blocks, but for those that do,
183+
/// we want to validate commands in the script block arguments.
184+
/// Unfortunately, we can't know how the argument is used. In the worst
185+
/// case, for commands like Get-ADUser, the script block actually
186+
/// specifies a different language.
187+
///
188+
/// Because we can't know ahead of time all of the commands that do
189+
/// odd things with script blocks, we create a white-list of commands
190+
/// that do invoke the script block - this covers the most useful cases.
162191
/// </summary>
163-
public Func<string, object> ValidationHandler { get; set; }
192+
public HashSet<string> CommandsToValidateScriptBlockArguments { get; set; }
164193

165194
/// <summary>
166195
/// When true, duplicates will not be added to the history.
@@ -386,17 +415,17 @@ public Func<string, bool> AddToHistoryHandler
386415

387416
[Parameter(ParameterSetName = "OptionsSet")]
388417
[AllowNull]
389-
public Func<string, object> ValidationHandler
418+
public Action<CommandAst> CommandValidationHandler
390419
{
391-
get { return _validationHandler; }
420+
get { return _commandValidationHandler; }
392421
set
393422
{
394-
_validationHandler = value;
395-
_validationHandlerSpecified = true;
423+
_commandValidationHandler = value;
424+
_commandValidationHandlerSpecified = true;
396425
}
397426
}
398-
private Func<string, object> _validationHandler;
399-
internal bool _validationHandlerSpecified;
427+
private Action<CommandAst> _commandValidationHandler;
428+
internal bool _commandValidationHandlerSpecified;
400429

401430
[Parameter(ParameterSetName = "OptionsSet")]
402431
public SwitchParameter HistorySearchCursorMovesToEnd

‎PSReadLine/Options.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ private void SetOptionsInternal(SetPSReadlineOption options)
5656
{
5757
Options.AddToHistoryHandler = options.AddToHistoryHandler;
5858
}
59-
if (options._validationHandlerSpecified)
59+
if (options._commandValidationHandlerSpecified)
6060
{
61-
Options.ValidationHandler = options.ValidationHandler;
61+
Options.CommandValidationHandler = options.CommandValidationHandler;
6262
}
6363
if (options._maximumHistoryCount.HasValue)
6464
{

‎PSReadLine/SamplePSReadlineProfile.ps1

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,3 +415,21 @@ Set-PSReadlineKeyHandler -Key Alt+j `
415415

416416
[PSConsoleUtilities.PSConsoleReadLine]::InvokePrompt()
417417
}
418+
419+
Set-PSReadlineOption -CommandValidationHandler {
420+
param([System.Management.Automation.Language.CommandAst]$CommandAst)
421+
422+
switch ($CommandAst.GetCommandName())
423+
{
424+
'git' {
425+
$gitCmd = $CommandAst.CommandElements[1].Extent
426+
switch ($gitCmd.Text)
427+
{
428+
'cmt' {
429+
[PSConsoleUtilities.PSConsoleReadLine]::Replace(
430+
$gitCmd.StartOffset, $gitCmd.EndOffset - $gitCmd.StartOffset, 'commit')
431+
}
432+
}
433+
}
434+
}
435+
}

‎PSReadLine/en-US/PSReadline.dll-help.xml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -567,11 +567,11 @@ Using the ScriptBlock parameter, one can achieve equivalent functionality by cal
567567
<command:parameterValue required="true" variableLength="false">Func[String, Boolean]</command:parameterValue>
568568
</command:parameter>
569569
<command:parameter required="false" variableLength="false" globbing="false" pipelineInput="false" position="named">
570-
<maml:name>ValidationHandler</maml:name>
570+
<maml:name>CommandValidationHandler</maml:name>
571571
<maml:description>
572-
<maml:para>Specifies a ScriptBlock that is called from ValidateAndAcceptLine. If a non-null object is returned or an exception is thrown, validation fails and the error is reported. If the object returned/thrown has a Message property, it's value is used in the error message, and if there is an Offset property, the cursor is moved to that offset after reporting the error. If there is no Message property, the ToString method is called to report the error.</maml:para>
572+
<maml:para>Specifies a ScriptBlock that is called from ValidateAndAcceptLine. If an exception is thrown, validation fails and the error is reported. Before throwing an exception, the validation handler may choose to place the cursor at the point of the error to make it easier to fix. A validation handler may also modify the command line, e.g. to correct common typos.</maml:para>
573573
</maml:description>
574-
<command:parameterValue required="true" variableLength="false">Func[String, Object]</command:parameterValue>
574+
<command:parameterValue required="true" variableLength="false">Action[CommandAst]</command:parameterValue>
575575
</command:parameter>
576576
<command:parameter required="false" variableLength="false" globbing="false" pipelineInput="false" position="named">
577577
<maml:name>HistorySearchCursorMovesToEnd</maml:name>
@@ -830,13 +830,13 @@ Using the ScriptBlock parameter, one can achieve equivalent functionality by cal
830830
<dev:defaultValue>None</dev:defaultValue>
831831
</command:parameter>
832832
<command:parameter required="false" variableLength="false" globbing="false" pipelineInput="false" position="named">
833-
<maml:name>ValidationHandler</maml:name>
833+
<maml:name>CommandValidationHandler</maml:name>
834834
<maml:description>
835-
<maml:para>Specifies a ScriptBlock that is called from ValidateAndAcceptLine. If a non-null object is returned or an exception is thrown, validation fails and the error is reported. If the object returned/thrown has a Message property, it's value is used in the error message, and if there is an Offset property, the cursor is moved to that offset after reporting the error. If there is no Message property, the ToString method is called to report the error.</maml:para>
835+
<maml:para>Specifies a ScriptBlock that is called from ValidateAndAcceptLine. If an exception is thrown, validation fails and the error is reported. Before throwing an exception, the validation handler may choose to place the cursor at the point of the error to make it easier to fix. A validation handler may also modify the command line, e.g. to correct common typos.</maml:para>
836836
</maml:description>
837-
<command:parameterValue required="true" variableLength="false">Func[String, Object]</command:parameterValue>
837+
<command:parameterValue required="true" variableLength="false">Action[CommandAst]</command:parameterValue>
838838
<dev:type>
839-
<maml:name>Func[String, String]</maml:name>
839+
<maml:name>Action[CommandAst]</maml:name>
840840
<maml:uri/>
841841
</dev:type>
842842
<dev:defaultValue>None</dev:defaultValue>

0 commit comments

Comments
 (0)