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.

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

Support for saving files in bundle #287

Open
wants to merge 1 commit into
base: feature/single-file-bundles
Choose a base branch
from

Conversation

CursedSheep
Copy link

Problem

As of commit 6d0f609, it does not support saving files inside the bundle.

Solution

Added an option in context menu to save files inside a bundle app.

Comment on lines +11 to +12
static readonly HashSet<char> invalidFileNameChar = new HashSet<char>();
static BundleNameCleaner() {
Copy link
Member

Choose a reason for hiding this comment

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

New empty line between method definition and field definition

Comment on lines +20 to +22
public static class SaveBundle {

/// <summary>
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary empty line here

Comment on lines +29 to +30
static bool CanExecute(AsmEditorContext context) => SaveBundleContentsCommand.IsSingleFileBundle(context);
static void Execute(AsmEditorContext context) {
Copy link
Member

Choose a reason for hiding this comment

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

New line between method with full body and method between statement body

Comment on lines +23 to +24
}
public override bool IsVisible(AsmEditorContext context) => SaveBundleContentsCommand.CanExecute(context);
Copy link
Member

Choose a reason for hiding this comment

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

New line between constructor and methods with statement bodies

Comment on lines +50 to +51
}
public override bool IsVisible(AsmEditorContext context) => SaveRawEntryCommand.CanExecute(context);
Copy link
Member

Choose a reason for hiding this comment

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

New line between constructor and methods with statement bodies

static bool IsSingleFileBundle(AsmEditorContext context) => context.Nodes.Length == 1 && context.Nodes[0] is BundleDocumentNode;
static bool CanExecute(AsmEditorContext context) => SaveBundleContentsCommand.IsSingleFileBundle(context);
static void Execute(AsmEditorContext context) {
var docNode = context.Nodes[0].GetDocumentNode();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var docNode = context.Nodes[0].GetDocumentNode();
var docNode = context.Nodes[0] as BundleDocumentNode;

We can just safe cast it as the CanExecute method has already checked whether the node is a BundleDocumentNode

static bool IsBundleSingleSelection(AsmEditorContext context) => context.Nodes.Length == 1 && context.Nodes[0] is IBundleEntryNode;
static bool CanExecute(AsmEditorContext context) => SaveRawEntryCommand.IsBundleSingleSelection(context);
static void Execute(AsmEditorContext context) {
var bundleEntryNode = (IBundleEntryNode)context.Nodes[0];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var bundleEntryNode = (IBundleEntryNode)context.Nodes[0];
var bundleEntryNode = context.Nodes[0] as IBundleEntryNode;
Debug2.Assert(bundleEntryNode is not null);

Use safe cast here and debug assert. Furthermore, please assert that bundleEntryNode.BundleEntry is not null

Comment on lines +33 to +34
Debug2.Assert(bundleDoc != null);
Debug2.Assert(bundleDoc.SingleFileBundle != null);
Copy link
Member

Choose a reason for hiding this comment

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

Use is not null instead of != null

static bool CanExecute(AsmEditorContext context) => SaveRawEntryCommand.IsBundleSingleSelection(context);
static void Execute(AsmEditorContext context) {
var bundleEntryNode = (IBundleEntryNode)context.Nodes[0];
SaveBundle.Save([bundleEntryNode.BundleEntry!], dnSpy_AsmEditor_Resources.SaveRawEntry);
Copy link
Member

Choose a reason for hiding this comment

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

Please use the old new BundleEntry[] { bundleEntryNode.BundleEntry } syntax instead

public override void Execute(AsmEditorContext context) => SaveRawEntryCommand.Execute(context);
}

static bool IsBundleSingleSelection(AsmEditorContext context) => context.Nodes.Length == 1 && context.Nodes[0] is IBundleEntryNode;
Copy link
Member

Choose a reason for hiding this comment

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

Check whether the IBundleEntryNode.BundleEntry is not null. Implementing this interface does not guarantee that the node has an entry!

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