Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Charp73 #173

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Charp73 #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
284493e
Update all projects to use C# 7.3
laurentkempe Aug 25, 2018
d75fc3b
Run R# default full cleanup
laurentkempe Aug 25, 2018
712fa20
Use nameof in place of string
laurentkempe Aug 25, 2018
165872d
Join null check with assignement
laurentkempe Aug 25, 2018
94ecf03
Use null propagation
laurentkempe Aug 25, 2018
ec22983
Inline out variable declaration
laurentkempe Aug 25, 2018
1e6cea5
Convert to ?: operator
laurentkempe Aug 25, 2018
5980b7a
Convert to return statement
laurentkempe Aug 25, 2018
0a5b293
Use pattern matching
laurentkempe Aug 25, 2018
f42936b
Convert delegate to local function
laurentkempe Aug 25, 2018
f160567
Introduce optional parameters
laurentkempe Aug 25, 2018
6a3891e
Invert if statement to reduce nesting
laurentkempe Aug 25, 2018
32aec68
Merge conditional ?: expression into contional access
laurentkempe Aug 25, 2018
5b15edd
Simplify code
laurentkempe Aug 25, 2018
b5bd9ce
Use string interpolation
laurentkempe Aug 25, 2018
4a89715
Remove useless ParseResultEventArgs class
laurentkempe Aug 25, 2018
dfaa74e
Delete unused local variable
laurentkempe Aug 25, 2018
77f43c5
Remove property Name which is never used
laurentkempe Aug 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions GitDiffMargin.Commands/CommandHandlerTextViewCreationListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
namespace GitDiffMargin.Commands
{
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Editor;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.TextManager.Interop;
using Microsoft.VisualStudio.Utilities;
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Editor;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.TextManager.Interop;
using Microsoft.VisualStudio.Utilities;

namespace GitDiffMargin.Commands
{
[Export(typeof(IVsTextViewCreationListener))]
[ContentType("text")]
[TextViewRole(PredefinedTextViewRoles.Editable)]
internal class CommandHandlerTextViewCreationListener : IVsTextViewCreationListener
{
private readonly SVsServiceProvider _serviceProvider;
private readonly IVsEditorAdaptersFactoryService _editorAdaptersFactoryService;
private readonly SVsServiceProvider _serviceProvider;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❔ Why was this reordered? Previously the fields, the constructor parameters, and the assignments in the constructor all appeared in the same order. Now they are inconsistent.


[ImportingConstructor]
public CommandHandlerTextViewCreationListener(SVsServiceProvider serviceProvider, IVsEditorAdaptersFactoryService editorAdaptersFactoryService)
public CommandHandlerTextViewCreationListener(SVsServiceProvider serviceProvider,
IVsEditorAdaptersFactoryService editorAdaptersFactoryService)
{
_serviceProvider = serviceProvider;
_editorAdaptersFactoryService = editorAdaptersFactoryService;
Expand All @@ -30,8 +31,8 @@ public void VsTextViewCreated(IVsTextView textViewAdapter)

// The new command handling approach does not require that the command filter be enabled. The command
// implementations interact directly with the handler via its IOleCommandTarget interface.
GitDiffMarginCommandHandler filter = new GitDiffMarginCommandHandler(textViewAdapter, _editorAdaptersFactoryService, textView);
var filter = new GitDiffMarginCommandHandler(textViewAdapter, _editorAdaptersFactoryService, textView);
textView.Properties.AddProperty(typeof(GitDiffMarginCommandHandler), filter);
}
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 I don't like how the missing newline at the end is presented by tools to look like an error.

12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/CopyOldTextCommandArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
namespace GitDiffMargin.Commands
{
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;

namespace GitDiffMargin.Commands
{
internal class CopyOldTextCommandArgs : EditorCommandArgs
{
public CopyOldTextCommandArgs(ITextView textView, ITextBuffer subjectBuffer)
: base(textView, subjectBuffer)
{
}
}
}
}
12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/CopyOldTextCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
namespace GitDiffMargin.Commands
{
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;

namespace GitDiffMargin.Commands
{
[Export(typeof(ICommandHandler))]
[ContentType("text")]
[Name(nameof(CopyOldTextCommandHandler))]
Expand All @@ -16,4 +16,4 @@ public CopyOldTextCommandHandler()

public override string DisplayName => "Copy Old Text";
}
}
}
1 change: 1 addition & 0 deletions GitDiffMargin.Commands/GitDiffMargin.Commands.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<DefineConstants>DEBUG;TRACE</DefineConstants>
<ErrorReport>prompt</ErrorReport>
<WarningLevel>4</WarningLevel>
<LangVersion>7.3</LangVersion>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<DebugType>pdbonly</DebugType>
Expand Down
25 changes: 15 additions & 10 deletions GitDiffMargin.Commands/GitDiffMarginCommandBinding.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,37 @@
namespace GitDiffMargin.Commands
{
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Editor.Commanding;
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Editor.Commanding;

namespace GitDiffMargin.Commands
{
internal class GitDiffMarginCommandBinding
{
#pragma warning disable CS0649 // Field 'fieldName' is never assigned to, and will always have its default value null

[Export]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint)GitDiffMarginCommand.PreviousChange, typeof(PreviousChangeCommandArgs))]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint) GitDiffMarginCommand.PreviousChange,
typeof(PreviousChangeCommandArgs))]
internal CommandBindingDefinition PreviousChangeCommandBinding;

[Export]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint)GitDiffMarginCommand.NextChange, typeof(NextChangeCommandArgs))]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint) GitDiffMarginCommand.NextChange,
typeof(NextChangeCommandArgs))]
internal CommandBindingDefinition NextChangeCommandBinding;

[Export]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint)GitDiffMarginCommand.RollbackChange, typeof(RollbackChangeCommandArgs))]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint) GitDiffMarginCommand.RollbackChange,
typeof(RollbackChangeCommandArgs))]
internal CommandBindingDefinition RollbackChangeCommandBinding;

[Export]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint)GitDiffMarginCommand.CopyOldText, typeof(CopyOldTextCommandArgs))]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint) GitDiffMarginCommand.CopyOldText,
typeof(CopyOldTextCommandArgs))]
internal CommandBindingDefinition CopyOldTextCommandBinding;

[Export]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint)GitDiffMarginCommand.ShowPopup, typeof(ShowPopupCommandArgs))]
[CommandBinding(GitDiffMarginCommandHandler.GitDiffMarginCommandSet, (uint) GitDiffMarginCommand.ShowPopup,
typeof(ShowPopupCommandArgs))]
internal CommandBindingDefinition ShowPopupCommandBinding;

#pragma warning restore CS0649 // Field 'fieldName' is never assigned to, and will always have its default value null
}
}
}
19 changes: 11 additions & 8 deletions GitDiffMargin.Commands/GitDiffMarginCommandHandler`1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
namespace GitDiffMargin.Commands
{
using System;
using Microsoft.VisualStudio.OLE.Interop;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using System;
using Microsoft.VisualStudio.OLE.Interop;
using Microsoft.VisualStudio.Text.Editor.Commanding;

namespace GitDiffMargin.Commands
{
internal abstract class GitDiffMarginCommandHandler<T> : ShimCommandHandler<T>
where T : EditorCommandArgs
{
protected GitDiffMarginCommandHandler(GitDiffMarginCommand commandId)
: base(new Guid(GitDiffMarginCommandHandler.GitDiffMarginCommandSet), (uint)commandId)
: base(new Guid(GitDiffMarginCommandHandler.GitDiffMarginCommandSet), (uint) commandId)
Copy link
Collaborator

@sharwell sharwell Aug 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Unexpected space after cast. If this is a style you want to use, you'll want to add it to a .editorconfig file because it's not the default behavior of Visual Studio.

{
}

protected override IOleCommandTarget GetCommandTarget(T args)
=> args.TextView.Properties.GetProperty<GitDiffMarginCommandHandler>(typeof(GitDiffMarginCommandHandler));
{
return args.TextView.Properties.GetProperty<GitDiffMarginCommandHandler>(
typeof(GitDiffMarginCommandHandler));
}
}
}
}
12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/NextChangeCommandArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
namespace GitDiffMargin.Commands
{
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;

namespace GitDiffMargin.Commands
{
internal class NextChangeCommandArgs : EditorCommandArgs
{
public NextChangeCommandArgs(ITextView textView, ITextBuffer subjectBuffer)
: base(textView, subjectBuffer)
{
}
}
}
}
12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/NextChangeCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
namespace GitDiffMargin.Commands
{
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;

namespace GitDiffMargin.Commands
{
[Export(typeof(ICommandHandler))]
[ContentType("text")]
[Name(nameof(NextChangeCommandHandler))]
Expand All @@ -16,4 +16,4 @@ public NextChangeCommandHandler()

public override string DisplayName => "Next Change";
}
}
}
12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/PreviousChangeCommandArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
namespace GitDiffMargin.Commands
{
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;

namespace GitDiffMargin.Commands
{
internal class PreviousChangeCommandArgs : EditorCommandArgs
{
public PreviousChangeCommandArgs(ITextView textView, ITextBuffer subjectBuffer)
: base(textView, subjectBuffer)
{
}
}
}
}
12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/PreviousChangeCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
namespace GitDiffMargin.Commands
{
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;

namespace GitDiffMargin.Commands
{
[Export(typeof(ICommandHandler))]
[ContentType("text")]
[Name(nameof(PreviousChangeCommandHandler))]
Expand All @@ -16,4 +16,4 @@ public PreviousChangeCommandHandler()

public override string DisplayName => "Previous Change";
}
}
}
3 changes: 1 addition & 2 deletions GitDiffMargin.Commands/Properties/AssemblyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

// General Information about an assembly is controlled through the following
Expand Down Expand Up @@ -34,4 +33,4 @@
// [assembly: AssemblyVersion("1.0.*")]
[assembly: AssemblyVersion("3.8.0.0")]
[assembly: AssemblyFileVersion("3.8.0.0")]
[assembly: AssemblyInformationalVersion("3.8.0.0")]
[assembly: AssemblyInformationalVersion("3.8.0.0")]
12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/RollbackChangeCommandArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
namespace GitDiffMargin.Commands
{
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;

namespace GitDiffMargin.Commands
{
internal class RollbackChangeCommandArgs : EditorCommandArgs
{
public RollbackChangeCommandArgs(ITextView textView, ITextBuffer subjectBuffer)
: base(textView, subjectBuffer)
{
}
}
}
}
12 changes: 6 additions & 6 deletions GitDiffMargin.Commands/RollbackChangeCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
namespace GitDiffMargin.Commands
{
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;
using System.ComponentModel.Composition;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Utilities;

namespace GitDiffMargin.Commands
{
[Export(typeof(ICommandHandler))]
[ContentType("text")]
[Name(nameof(RollbackChangeCommandHandler))]
Expand All @@ -16,4 +16,4 @@ public RollbackChangeCommandHandler()

public override string DisplayName => "Rollback Change";
}
}
}
41 changes: 18 additions & 23 deletions GitDiffMargin.Commands/ShimCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,52 +1,47 @@
namespace GitDiffMargin.Commands
{
using System;
using Microsoft.VisualStudio;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Shell;
using IOleCommandTarget = Microsoft.VisualStudio.OLE.Interop.IOleCommandTarget;
using OLECMD = Microsoft.VisualStudio.OLE.Interop.OLECMD;
using OLECMDEXECOPT = Microsoft.VisualStudio.OLE.Interop.OLECMDEXECOPT;
using OLECMDF = Microsoft.VisualStudio.OLE.Interop.OLECMDF;
using System;
using Microsoft.VisualStudio;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.OLE.Interop;
using Microsoft.VisualStudio.Shell;

namespace GitDiffMargin.Commands
{
internal abstract class ShimCommandHandler<T> : ICommandHandler<T>
where T : CommandArgs
{
private readonly Guid _commandSet;
private readonly uint _commandId;
private readonly Guid _commandSet;

protected ShimCommandHandler(Guid commandSet, uint commandId)
{
_commandSet = commandSet;
_commandId = commandId;
}

public abstract string DisplayName
{
get;
}
public abstract string DisplayName { get; }

public virtual CommandState GetCommandState(T args)
{
ThreadHelper.ThrowIfNotOnUIThread();

OLECMD[] command = { new OLECMD { cmdID = _commandId } };
OLECMD[] command = {new OLECMD {cmdID = _commandId}};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Unexpected removal of spaces inside braces

ErrorHandler.ThrowOnFailure(GetCommandTarget(args).QueryStatus(_commandSet, 1, command, IntPtr.Zero));
if ((command[0].cmdf & (uint)OLECMDF.OLECMDF_SUPPORTED) == 0)
if ((command[0].cmdf & (uint) OLECMDF.OLECMDF_SUPPORTED) == 0)
return CommandState.Unspecified;
else if ((command[0].cmdf & (uint)OLECMDF.OLECMDF_ENABLED) == 0)
return CommandState.Unavailable;
else
return CommandState.Available;

return (command[0].cmdf & (uint) OLECMDF.OLECMDF_ENABLED) == 0
? CommandState.Unavailable
: CommandState.Available;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 This is not wrong, but I don't like how some of this method uses an if with early return, while another part uses a ternary. Given the complexity of the condition expressions, I believe the original form was preferable.

}

public virtual bool ExecuteCommand(T args, CommandExecutionContext executionContext)
{
ThreadHelper.ThrowIfNotOnUIThread();

return ErrorHandler.Succeeded(GetCommandTarget(args).Exec(_commandSet, _commandId, (uint)OLECMDEXECOPT.OLECMDEXECOPT_DODEFAULT, IntPtr.Zero, IntPtr.Zero));
return ErrorHandler.Succeeded(GetCommandTarget(args).Exec(_commandSet, _commandId,
(uint) OLECMDEXECOPT.OLECMDEXECOPT_DODEFAULT, IntPtr.Zero, IntPtr.Zero));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 I dislike how some tools seem to think code should read like a book. One of the smartest rules I've observed in StyleCop is the part of rules that says all arguments should be on one line, or each argument should be on a line of its own.

}

protected abstract IOleCommandTarget GetCommandTarget(T args);
}
}
}
Loading