These are chat archives for AvaloniaUI/Avalonia

16th
Nov 2016
José Manuel Nieto
@SuperJMN
Nov 16 2016 10:20
The Content is set
danwalmsley
@danwalmsley
Nov 16 2016 10:33
@SuperJMN yes but when you set it, does it fire the event?
and pass the AvaloniaPropertyEventArgs to avalonia?
@jkoritzinsky do I remember rightly we talked once about supporting HitTestVisible=false for popup?
José Manuel Nieto
@SuperJMN
Nov 16 2016 10:50
@danwalmsley I've put a breakpoint in AvaloniaObject.OnPropertyChanged and it's not hit
Steven Kirk
@grokys
Nov 16 2016 11:33
@SuperJMN hmm very strange, it really sounds like styles aren't present. I will hopefully have some time to take a look today
José Manuel Nieto
@SuperJMN
Nov 16 2016 11:42
OK, thanks
danwalmsley
@danwalmsley
Nov 16 2016 11:46
has anyone run avalonia on Linux gtk recently?
danwalmsley
@danwalmsley
Nov 16 2016 11:51
running control catalog on Linux is trying to use...
gtk-win32.dll
danwalmsley
@danwalmsley
Nov 16 2016 12:14
we are definitely broken on Linux! not sure if we have recent changes to gtk / Linux backend?
Steven Kirk
@grokys
Nov 16 2016 12:27
oh really? i can't think of any recent changes that would have caused that
danwalmsley
@danwalmsley
Nov 16 2016 12:38
maybe that was the controlcatalog that I tried from the artificats produced by appveyor
iv tried running Avalon studio on Linux now
it runs...
but doesn't render correctly at all
I end up with a white window, with a square in the middle!
I'm going to nudge @jameswalmsley on the skia backend integration on Linux again,,, see if that works
@grokys did you have any comments on
#805?
Steven Kirk
@grokys
Nov 16 2016 14:02
not had chance to look at it in details yet - be assured its on my backlog
which is at this point quite large
danwalmsley
@danwalmsley
Nov 16 2016 14:08
no worries,
are there any PRs you would like me to checkout ?
Steven Kirk
@grokys
Nov 16 2016 14:59
any that are open would be a massive help!
danwalmsley
@danwalmsley
Nov 16 2016 15:57
@grokys if you give me access to branch 736-removetestapplication I can push commit that resolves conflicts
Steven Kirk
@grokys
Nov 16 2016 16:01
you should already have access to that branch - it's in the main repository
there's no such thing as "access to a branch" in git - it's by repository
danwalmsley
@danwalmsley
Nov 16 2016 16:03
oh it did push
I got unauthorized on my screen!
wierd
Steven Kirk
@grokys
Nov 16 2016 16:10
weird... i think GH was having problems a little while ago, might have been that
danwalmsley
@danwalmsley
Nov 16 2016 16:15
probably me, not had much sleep last few days!
Steven Kirk
@grokys
Nov 16 2016 16:17
no, i think it was GH - i've seen some other reports of pushes failing
danwalmsley
@danwalmsley
Nov 16 2016 16:20
travis is getting really slow
it would be ok apart from mac osx builds
I had one have to wait for several hours the other day
Steven Kirk
@grokys
Nov 16 2016 16:24
yep, it's terrible. the paid accounts aren't much better. i've heard people talking about moving to CircleCI, but i don't know if that's much better
danwalmsley
@danwalmsley
Nov 16 2016 16:25
i guess which ever ci platform the crowd jumps to ends up getting swamped
José Manuel Nieto
@SuperJMN
Nov 16 2016 18:06
Hey @grokys! Have you checked the issue?
Steven Kirk
@grokys
Nov 16 2016 18:08
no sorry, i've been quite busy these last few days
Steven Kirk
@grokys
Nov 16 2016 18:25
hopefully will have time for you and @danwalmsley in an hour or so
José Manuel Nieto
@SuperJMN
Nov 16 2016 18:28
woohoo!!
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 18:46
I've been looking at some stuff about doing interactive testing (so we can actually test Win32 and GTK windowing toolkits). Looks like AppVeyor runs build agents as interactive processes so we already can run Win32 tests there, and we can use xvfb on Travis to run UI tests for GTK. Is this something we might want to pursue in the future?
danwalmsley
@danwalmsley
Nov 16 2016 18:52
@jkoritzinsky did you see my question about hittestvisible=false on popup?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 18:53
Right... I haven't looked at that in a while.
If I remember correctly I found a way to do it in Win32, but I don't remember how and I'm having trouble finding it again with just a quick look.
I'll let you know if I can find it a second time
danwalmsley
@danwalmsley
Nov 16 2016 18:55
Ok, in theory if it was disabled it shouldn't be hittested
So il see if that might do what I need
danwalmsley
@danwalmsley
Nov 16 2016 19:20
Iv been waiting 1.5 hours for 1 build to go through Travis now
Do we need the mac osx ci at the moment?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:23
@grokys you work at GH right? Any inside info on if a "Complete when Green" feature isin the process of being built? We could really use that right now (and a lot of FH competitors have it)
Steven Kirk
@grokys
Nov 16 2016 19:23
you mean to merge automatically when the CI turns green?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:24
Yep
Steven Kirk
@grokys
Nov 16 2016 19:24
i've not heard of anything, but it's a good feature request!
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:24
I can say that once we implemented that in VSTS we used it literally all the time.
And your competitor GitLab has it too
Steven Kirk
@grokys
Nov 16 2016 19:26
i will suggest it!
though i'm sure they're aware
and if it were in the works i probably couldn't confirm it anyway ;)
(btw that's not a confirmation, I have no idea) ;)
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:27
Haha I know
danwalmsley
@danwalmsley
Nov 16 2016 19:27
Can we allow it to merge without it forcing us to update to master
That might reduce bottle neck
Steven Kirk
@grokys
Nov 16 2016 19:28
yeah, i don't think that would be an accepted feature though, as it could break stuff if the merge gets stuff wrong (as it frequently does with e.g. .sln files)
so you'd always want to run CI anyway
danwalmsley
@danwalmsley
Nov 16 2016 19:31
Sure,
Gitlab does it, but ov also done merges on it and it's completely messed up
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:34
You could also manually pull the merge commit and update your master to point to it and push it out.
Definitely requires more knowledge of how to use git internally, but should work
Unless we block direct (non-force) pushes to master, which I've never tried.
Steven Kirk
@grokys
Nov 16 2016 19:35
yeah, pushes to master are blocked
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:35
Ok so that won't work. It was a good idea though
Steven Kirk
@grokys
Nov 16 2016 19:35
using the protected branches feature
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:36
I'm looking at the features now. It says that it only blocks force pushes
Steven Kirk
@grokys
Nov 16 2016 19:36
maybe we should make travis on mac block PRs
@jkoritzinsky try it!
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:36
Which PR do you want me to try with?
@danwalmsley Which one are you waiting on?
Steven Kirk
@grokys
Nov 16 2016 19:36
oh i meant just commit to master and try pushing
ok @SuperJMN taking a look at your problem now
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:38
thank you!
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:38
@grokys I don't have anything I need to commit directly...
danwalmsley
@danwalmsley
Nov 16 2016 19:39
@jkoritzinsky #797 ita finally gone through, are you happy with the pr, I'm going to test it with AvalonStudio and if it works fine ill merge
Trying to clear pr backlog a bit
Steven Kirk
@grokys
Nov 16 2016 19:39
@jkoritzinsky well that's good because you can't ;)
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:39
Yep! It's ready for merge when you're ready
@grokys Haha ok. I'm glad we checked though
Steven Kirk
@grokys
Nov 16 2016 19:40
thanks for helping out with PRs guys
now that VS2017 RC is out i should have some more time
we've been scrambling to get everything in order for that
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:40
Working on the "GH4VS" integration?
Steven Kirk
@grokys
Nov 16 2016 19:40
yeah...
we've had to completely change the way our package is loaded as they're now timing extension startup times
so had to async-ify a ton of things, and add new startpage functionaly
but the worst thing is i couldn't talk about it as it was all NDA
danwalmsley
@danwalmsley
Nov 16 2016 19:42
@grokys does the rc have support for csproj tooling for new .net core?
Steven Kirk
@grokys
Nov 16 2016 19:42
so when i go quiet, assume something like that ;)
@danwalmsley not had chance to try that yet
danwalmsley
@danwalmsley
Nov 16 2016 19:42
getting it downloaded asap!
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:42
@grokys At a conference I was at when in Seattle during my internship they were talking about the startup time limitations
Makes sense that you were effected by that
Steven Kirk
@grokys
Nov 16 2016 19:43
yeah
tbh i'm not sure how useful it is because you can just defer your slowness until after package startup
but it's a start i guess
but developing an OSS product under NDA is a real PITA
danwalmsley
@danwalmsley
Nov 16 2016 19:44
@jkoritzinsky to test your PR, I should just be able to set <Image Source=... .ico file />?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:45
No. That should already work without my PR.
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:45
@grokys just to give you a place to look, maybe the problem is related to the assignment of the values (it happens inside AvaloniaObjectBuilder.PerformAssigment)
whenever a value is going to be assigned, it goes there
Steven Kirk
@grokys
Nov 16 2016 19:46
hmm @SuperJMN i'm getting errors like:
Severity    Code    Description    Project    File    Line    Suppression State
Error    CS0246    The type or namespace name 'Assembly' could not be found (are you missing a using directive or an assembly reference?)    OmniXamlv1    D:\projects\Avalonia-JMN\src\Markup\OmniXAMLv1\Builder\ConfiguredAssemblyWithNamespaces.cs    10    Active
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:47
arghg
danwalmsley
@danwalmsley
Nov 16 2016 19:47
@grokys from what you have seen, has visual studio completely changed? or is it just a new version?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:47

You would need to do something like the following:

<Window Name="Win" Icon="icon.ico"><Image Source="{Binding #Win.Icon, Converter={YourConverter}}" /> </Window>

And YourConverter is an IValueConverter that does something similar to new Bitmap(icon.Save())

José Manuel Nieto
@SuperJMN
Nov 16 2016 19:47
I will clone it and see if the solution is missing something
Steven Kirk
@grokys
Nov 16 2016 19:47
@danwalmsley no it's pretty much the same - some nice tweaks here and there but nothing huge so far
danwalmsley
@danwalmsley
Nov 16 2016 19:48
@jkoritzinsky ok just testing now
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:48
@danwalmsley It's slimmer and loads faster and easier to change what features you have installed, but other than that it's still plain old visual studio
danwalmsley
@danwalmsley
Nov 16 2016 19:48
@jkoritzinsky @grokys that's cool, I'm sure if they had re-written it that "Visual Studio for Mac" would have been part of it
Steven Kirk
@grokys
Nov 16 2016 19:48
@SuperJMN i can't understand how it can't find Assembly!
danwalmsley
@danwalmsley
Nov 16 2016 19:48
which would be pretty awesome
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:49
@grokys that's nasty!
Steven Kirk
@grokys
Nov 16 2016 19:51
@SuperJMN there are a bunch of unresolved references
blob
danwalmsley
@danwalmsley
Nov 16 2016 19:52
I don't mean to state the obvious, did you remember to update submodules?
I had a similar thing when I tried out this yesterday
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:52
oh my...
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:52
And restore nuget packages?
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:53
I'll try to resolve it
Steven Kirk
@grokys
Nov 16 2016 19:53
aggh
damn you're probably right @danwalmsley
how many times have i told other people to do that?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:54
This situation is why I don't like submodules
Steven Kirk
@grokys
Nov 16 2016 19:54
hmm, still can't find Assembly
looks like .net standard problem
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:55
Long term feature request: Once OmniXAML v2 is stable, can we depend on a nuget package version instead of submodules?
danwalmsley
@danwalmsley
Nov 16 2016 19:55
@jkoritzinsky yeah, do you know of any alternatives to submodules?
or a way to make them less painful?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:56
Basically we just need to get nuget packages of everything we use. I don't know any other good alternatives
We can always host internal versions of packages like OmniXAML that aren't currently available and work with @SuperJMN to publish simultaneously
Steven Kirk
@grokys
Nov 16 2016 19:56
yeah, we were only using a submodule for omnixaml because in the early days i was making changes to it
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:57
That's a perfect reason to have it as a submodule
Steven Kirk
@grokys
Nov 16 2016 19:57
and it was easier to test in place and commit to a fork and PR from there
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:57
Like that's literally the use case for submodules
Steven Kirk
@grokys
Nov 16 2016 19:57
yeah
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:57
@grokys it see that after cloning with VS, the OmniXAMLv2 submodule is not initialized
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 19:57
As we move away from that we should try to minimize our submodule usage
José Manuel Nieto
@SuperJMN
Nov 16 2016 19:57
@grokys do you know how to initialize it?
Steven Kirk
@grokys
Nov 16 2016 19:58
yeah i've updated submodules now
i'm not sure if this is the problem but after building i have:
blob
danwalmsley
@danwalmsley
Nov 16 2016 20:00
@grokys i got exactly the same thing as you yesterday
i just discarded them
no idea why it happens though
i also had to delete some folders before it would clone sub modules
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:00
discard them and see what happens
I'm building as we talk
Steven Kirk
@grokys
Nov 16 2016 20:02
ok, fixed the missing Assembly by including the System.Reflection nuget package
still not building though
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:02
that's strange. I've cloned it and I have it already executing :(
try to compile Glass first
the projects inside Markup\OmniXAMLv2 to see what happens. They should build.
Steven Kirk
@grokys
Nov 16 2016 20:03
ah "Glass" was unloaded
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:04
you're not happy with Glass, are you? :P
Steven Kirk
@grokys
Nov 16 2016 20:04
ha why do you say that?
having said that, no ;)
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:05
hahaha
Glass is always in the middle, causing harm and dead bodies to shudder under their tombs
Steven Kirk
@grokys
Nov 16 2016 20:06
not really a fan of "miscellaneous" library dependencies
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:06
unfortunately, it's used by several other of my libs :P
but this time, is inside OmniXAML, because it evolves too quickly
Steven Kirk
@grokys
Nov 16 2016 20:07
ok, it's running now!
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:07
(and paired with OXAML)
Steven Kirk
@grokys
Nov 16 2016 20:07
not sure why i had to add the System.Reflection nuget but you didn't
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:07
oh man, you're going to see OmniXAML v2 in action, and... its miseries
I don't remember having to add that package in my whole life
things change fast in the .NET world
Steven Kirk
@grokys
Nov 16 2016 20:08
btw has anyone else noticed a massive delay when shutting down avalonia applications recently? or is that just my machine?
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:09
not my case, at least the in the app I'm testing
Steven Kirk
@grokys
Nov 16 2016 20:11
@SuperJMN how does the XAML get loaded here? there's no InitializeComponent
ah, np, i see
danwalmsley
@danwalmsley
Nov 16 2016 20:11
btw has anyone else noticed a massive delay when shutting down avalonia applications recently? or is that just my machine?
yes
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:11
Everything is inside App.xaml.cs
Steven Kirk
@grokys
Nov 16 2016 20:11
you're loading it in App.xaml
yeah just saw
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:12
It's a temporary thingy 😅
Steven Kirk
@grokys
Nov 16 2016 20:20
ok, it's because when it comes to apply this Template to the Window:
the Setter.Value is null
so looks like omnixaml isn't deserializing the template
@SuperJMN to debug it add this code:
if (control.GetType().Name == "Window" && ((Setter)setter).Property.Name == "Template")
{
    System.Diagnostics.Debugger.Break();
}
you will see that setter.Value is null
danwalmsley
@danwalmsley
Nov 16 2016 20:29
@jkoritzinsky
Icon="icon.ico"><Image Source="{Binding #Win.Icon, Converter={YourConverter}}" />
how does it find YourConverter?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 20:30
That's just a placeholder for you to put in however you would find the converter
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:30
@grokys I'll take a look!
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 20:31
Like using {Static IconValueConverter.Converter} or something
danwalmsley
@danwalmsley
Nov 16 2016 20:31
ok
also what type would the icon be?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 20:31
WindowIcon I believe
Steven Kirk
@grokys
Nov 16 2016 20:34
@danwalmsley added some comments to the PR, just small things
but i wanted to talk about naming of the property
i know you took it from MahApps but it's not very descriptive to me: IgnoreTaskBarOnMaximize
what does "ignore" mean? even after reading the code and the comments i'm not sure
danwalmsley
@danwalmsley
Nov 16 2016 20:35
class IconImageConverter : IValueConverter
    {
        private static IconImageConverter s_converter = new IconImageConverter();

        public static IconImageConverter Converter => s_converter;

        public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
        {
            if (value is WindowIcon)
            {
                return new Bitmap((value as WindowIcon).Save());
            }

            return null;
        }
@grokys yeh I didn't think it was very good
Steven Kirk
@grokys
Nov 16 2016 20:36
i mean at first glace of the property name it looks like you'd use it to ignore task switching which is obviously not the case
danwalmsley
@danwalmsley
Nov 16 2016 20:36
it basically means (only when HasSystemDecorations=false)
maximized either covers taskbar
or not
Steven Kirk
@grokys
Nov 16 2016 20:36
and even knowing what it does, does "ignore" mean "cover the taskbar" or "don't cover the taskbar"?
danwalmsley
@danwalmsley
Nov 16 2016 20:36
ignore means not cover taskbar
Steven Kirk
@grokys
Nov 16 2016 20:36
ok
danwalmsley
@danwalmsley
Nov 16 2016 20:36
I'm trying to think of a better name
Steven Kirk
@grokys
Nov 16 2016 20:37
yeah, me too
danwalmsley
@danwalmsley
Nov 16 2016 20:37
MaximizeMode=Screen
MaximizeMode=Desktop
Steven Kirk
@grokys
Nov 16 2016 20:37
naming is hard...
CoverTaskbarOnMaximize?
danwalmsley
@danwalmsley
Nov 16 2016 20:37
that's better isn't it
CoverTaskbarOnMaximize
Steven Kirk
@grokys
Nov 16 2016 20:38
CoverTaskbarOnMaximizeWhenSystemDecorationsIsFalse?
danwalmsley
@danwalmsley
Nov 16 2016 20:38
haha
Steven Kirk
@grokys
Nov 16 2016 20:38
it's rather confusing
danwalmsley
@danwalmsley
Nov 16 2016 20:38
and it kind of only applies to Windows
do you think CoverTaskbarOnMaximize is ok? I don't think it will be a widely used feature
Steven Kirk
@grokys
Nov 16 2016 20:39
yeah i wanted to expose platform-specific things like this on IWindowImpl - so you'd cast Window.PlatformImpl to Win32.WindowImpl and set a property there
but @kekekeks made WindowImpl internal at some point, meaning we can't do that
danwalmsley
@danwalmsley
Nov 16 2016 20:40
I suppose it might be possible to make the Linux windows fully cover entire screen though I wouldn't have a clue how to do that
Steven Kirk
@grokys
Nov 16 2016 20:40
yeah... i'm not sure
we could make WindowImpl public again and add it back there?
danwalmsley
@danwalmsley
Nov 16 2016 20:40
might it be posslbe to do something like this, perhaps attached properties.
Steven Kirk
@grokys
Nov 16 2016 20:41
that might be an idea
Win32.CoverTaskbarOnMaximize="true"
danwalmsley
@danwalmsley
Nov 16 2016 20:41

<Window .....
    <Window.Win32.FullScreenMode="Desktop" /> 
</Window>
Steven Kirk
@grokys
Nov 16 2016 20:42
yeah, that might be even better
oh, you edited it
i kinda liked it before ;)
though it'd be <Window Win32.FullScreenMode="Desktop">
danwalmsley
@danwalmsley
Nov 16 2016 20:43
would we run into problems if we run that on Linux or mac machine?
how would it know to ignore it?
Steven Kirk
@grokys
Nov 16 2016 20:43
hmm, yeah that would be a problem
danwalmsley
@danwalmsley
Nov 16 2016 20:44
I wonder if we can make it not crash if you try to set a property that doesn't exist, it just ignores it
or at least enable that for some properties?
Steven Kirk
@grokys
Nov 16 2016 20:44
hmm
but how would you know which properties?
danwalmsley
@danwalmsley
Nov 16 2016 20:45
when you declare a property, RegisterAttached( optional = true)
oh I see
Steven Kirk
@grokys
Nov 16 2016 20:45
;)
danwalmsley
@danwalmsley
Nov 16 2016 20:45
you don't have the object ahead of time
so you cant know
you can only enable this scenario for everything or nothing
Steven Kirk
@grokys
Nov 16 2016 20:46
the problem is that it's not registered ;)
if you were registering it, it wouldn't be a problem
maybe for now, just find a better name for the property and add a "TODO: we need to rethink platform-specific functionality for 1.0"
although, we already have some things that won't be supported on certain platforms (like i think opacity masks? maybe)
not sure
for now lets think of a decent name for the property and Lets Get This Merged ( (c) @Seeker1437 )
danwalmsley
@danwalmsley
Nov 16 2016 20:52
ok just about to merge @jkoritzinsky pr first if I can get it to work
then will respond to your comments
Steven Kirk
@grokys
Nov 16 2016 20:52
np ;)
danwalmsley
@danwalmsley
Nov 16 2016 20:52
and great! ;)
@jkoritzinsky merged your pr ;)
works perfectly
José Manuel Nieto
@SuperJMN
Nov 16 2016 20:59
@grokys I've found the problem!!! THANKS!
Steven Kirk
@grokys
Nov 16 2016 20:59
cool :)
@jkoritzinsky just got chance to look at the PR that @danwalmsley merged. the Stream Save() method is a bit of an unusual pattern
should they use the same pattern?
Save(Stream) is the usual way of doing it, e.g. https://msdn.microsoft.com/en-us/library/ms142147(v=vs.110).aspx
danwalmsley
@danwalmsley
Nov 16 2016 21:21
@grokys iv updated PR, I think it now addresses the points your raised
let me know if there is anything else
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:24
We could change the pattern. I've personally never liked that pattern but I can change it to that if you want
Steven Kirk
@grokys
Nov 16 2016 21:26
well the pattern is useful in that you can e.g. open a file stream and write to that
what is it that you don't like?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:27
It just seems that every time I try to use it I always want it to return a stream. Probably because I'm only doing in-memory stuff (usually related to icons).
I don't know why but I just never seem to remember it correctly.
Steven Kirk
@grokys
Nov 16 2016 21:29
hmm, whenever i save things i usually want to write to a stream that i've opened myself
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:29
I'm usually using the saved stream to construct something else.
Maybe that's just because the only raw work ive done with Bitmaps is converting between Bitmaps and Icons
I've been working on a weird use case in comparison to the usual one.
I agree that it should be changed.
danwalmsley
@danwalmsley
Nov 16 2016 21:30
@grokys wooohoooo!
vs2017 has the new tooling...
image.png
does that mean we can migrate soon?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:31
I feel like maybe calling the method SaveTo(Stream stream) would sound the most intuitive to me
Johan Larsson
@JohanLarsson
Nov 16 2016 21:31
not CopyTo?
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:32
I'm just going off the original name Save(Stream stream)
Steven Kirk
@grokys
Nov 16 2016 21:32
elsewhere uses Save(Stream) - it's a common pattern in .net - i think we should follow it
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:32
@danwalmsley Let's wait until we know the tooling works in VS2015 or VS2017 is released so we can develop on a stable IDE version
@grokys Let's follow the pattern. I agree
danwalmsley
@danwalmsley
Nov 16 2016 21:32
@jkoritzinsky sure.... I'm just impatient! I really want to see this working on .net core ;)
Johan Larsson
@JohanLarsson
Nov 16 2016 21:33
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:33
@danwalmsley See if you can get that to open in VS2015
danwalmsley
@danwalmsley
Nov 16 2016 21:33
@jkoritzinsky ok
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:33
If you can, then let's upgrade.
danwalmsley
@danwalmsley
Nov 16 2016 21:33
we can always have it in on a separate branch for now
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:34
True true
Go for it if you're feeling up to it.
danwalmsley
@danwalmsley
Nov 16 2016 21:34
I might
probably not for a week or so
i'll have to find some more time!
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:35
I don't think it will work in VS2015. The project system is built on CPS which was in beta in VS2015 and has API incompatibilities and less features than its VS2017 counterpart.
danwalmsley
@danwalmsley
Nov 16 2016 21:36
image.png
it opened ... but missing references
so prob wont work for now
Johan Larsson
@JohanLarsson
Nov 16 2016 21:40
ok, sry, I kneejerked :)
Jeremy Koritzinsky
@jkoritzinsky
Nov 16 2016 21:40
@grokys Can you open an issue for the API change? I'm pretty busy with schoolwork so I don't have time to make that change right now.
Steven Kirk
@grokys
Nov 16 2016 21:41
sure!
@JohanLarsson np ;)
danwalmsley
@danwalmsley
Nov 16 2016 21:51
@grokys the indentation...
image.png
is that ok?
or this...
image.png
Steven Kirk
@grokys
Nov 16 2016 21:58
@danwalmsley yep, the second - that's how stuff's indented elsewhere
danwalmsley
@danwalmsley
Nov 16 2016 22:00
ok good that's what iv just pushed
Darnell Williams
@Seeker1437
Nov 16 2016 22:00
I now understand why older code uses the fully qualified namespace name.
Now not name lol
danwalmsley
@danwalmsley
Nov 16 2016 22:00
:)
Darnell Williams
@Seeker1437
Nov 16 2016 22:00
Because I guess .net didnt have a way to do this automatically in the past xD
When I downgrade to .net3.5 thats when I learned lol
My tool must target all the platforms the tools topic application covers meaning it reaches xp ug
Let's Get This Merged :D
danwalmsley
@danwalmsley
Nov 16 2016 22:02
:)
Steven Kirk
@grokys
Nov 16 2016 22:13
thanks Dan!
feel free to merge when CI passes if I don't get to it first ;)
Darnell Williams
@Seeker1437
Nov 16 2016 22:15
muhahahahahaha
danwalmsley
@danwalmsley
Nov 16 2016 22:18
sure... in about 2 hours ;)
image.png
danwalmsley
@danwalmsley
Nov 16 2016 22:24
travis has a backlog of 1000 macosx builds!