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

Fix for issue #1785 #1786

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix for issue #1785 #1786

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 27, 2015

This change allows bundles to be created from selections that contain files other than the type being bundled, as long as there are at least two files of the correct type selected.

Only the files of the correct type will actually be added to the bundle, all others will be ignored..

menuCommand.Enabled = _files.All(file => extensions.Contains(Path.GetExtension(file))) &&
_files.Count() > 1;
menuCommand.Enabled = _files.Any(file => extensions.Contains(Path.GetExtension(file))) &&
_files.Count(file => extensions.Contains(Path.GetExtension(file))) > 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all this can be replaced by one line:

menuCommand.Enabled = _files.Any(file => extensions.Contains(Path.GetExtension(file)));

Is it not what we are trying to say here?

Copy link
Author

Choose a reason for hiding this comment

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

I kept the original logic requiring that at least 2 files be required to create a bundle, but I have no problem with dropping this requirement so that single-file bundles can be created from the menu.

This might actually be useful in the case the you intend to start referencing the bundle immediately but add more files later.

Do I need to create a new pull request for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I forgot about that! :)

So this will become:

menuCommand.Enabled = _files.Count(file => extensions.Contains(Path.GetExtension(file))) > 1;

Do I need to create a new pull request for this?

No. If you are using command line git, you can do the following:

# after making all the changes
cd into\WE2013\directory
git commit --amend
# close the notepad, or you can change the commit message there save and then close
git push -f
# to rewrite the commit history of your fork's branch
# this PR will automatically acquire your fork changes made in this particular branch

A pull request (PR) is always attached to your branch (in this case your master branch), as far as it is opened. So any changes you will make in that branch meanwhile, will be reflected in the PR.

@am11
Copy link
Contributor

am11 commented Feb 28, 2015

Nice work locating the bug and fixing it. 👍

Please make sure that it does not break the feature: #1349.

@madskristensen
Copy link
Owner

Let me know when this is ready to merge

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.

3 participants