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

Solves issue #5 by looking at parent ProjectItem insteed of proj root #6

Merged
merged 4 commits into from Jan 19, 2019

Conversation

@enkelmedia
Copy link

@enkelmedia enkelmedia commented Apr 4, 2018

I this that this might be a solution to the issue that the preview does not work for files thats not in the root of the project

@enkelmedia
Copy link
Author

@enkelmedia enkelmedia commented Jun 26, 2018

No comments of feedback on this one? I've used it in my VS install for over 3 months without any problems

@golavr golavr self-assigned this Dec 10, 2018
@golavr golavr added this to the v3.3 milestone Dec 10, 2018
Copy link
Owner

@golavr golavr left a comment

Thanks for contributing
I'm sorry i couldn't respond quicker
Please review the comment i left you
To test it just open plain simple console application

  1. Right click on app.config
  2. Select Preview Config Transform
var containingProject = source.ContainingProject;
if (containingProject == null) return null;
// getting the parent of the clicked project item
var parent = (ProjectItem) source.Collection.Parent;

This comment has been minimized.

@golavr

golavr Dec 10, 2018
Owner

What if the source.Collection.Parentis Project and not ProjectItem?
You are going to get cast exception.

@enkelmedia
Copy link
Author

@enkelmedia enkelmedia commented Dec 11, 2018

Hi!

No worries :) I use this extension all the time :)

Hmm, why would you look at a transform for a config-file without a parent? Like in your example? Maybe we should just catch an exception like that and show some massage box? Or what are you thinking?

Edit: What I mean is that if the -config-files are correctly nested the parent will always be the ”root” config file? Just running a preview on app.config does not really make sense?

@golavr
Copy link
Owner

@golavr golavr commented Dec 11, 2018

i didn't say w/o parent :)
i'm saying when parent of the selected item is Project which is the case when you right click app.config in console application
var parent = (ProjectItem) source.Collection.Parent

@enkelmedia
Copy link
Author

@enkelmedia enkelmedia commented Dec 11, 2018

@golavr I understand, thats wrong. But why would someone want to run the preview on the "raw" app.config? How would you think that it should be handled?

Edit: Should I show an Msgbox with an error in that case? Or what would you say?

@enkelmedia
Copy link
Author

@enkelmedia enkelmedia commented Jan 11, 2019

I've updated the code to you example and it seems to work fine now.

@golavr
Copy link
Owner

@golavr golavr commented Jan 18, 2019

@enkelmedia thanks for the fix
we have 2 failed unit tests
please try to fix them and i'll merge it to master

@enkelmedia
Copy link
Author

@enkelmedia enkelmedia commented Jan 18, 2019

I've updated the PR, since the test was written with only the "Project" as parent in mind I had to rewrite them a little. Now they are checking that a parent Project returns null and a parent ProjectItem actually comes back correct.

@golavr
Copy link
Owner

@golavr golavr commented Jan 19, 2019

thank you @enkelmedia 👍

@golavr golavr merged commit db2e285 into golavr:master Jan 19, 2019
2 checks passed
2 checks passed
configuration-transform-gated-pipeline #20190118.1 succeeded
Details
golavr.ConfigurationTransform Build #20190118.2 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.