Add IDisposable to Glib.Dir

[fleimeris]

Fixes #521

[badcel]

Thank you for your contribution.

I will hopefully be able to review it later today.

[badcel]

Thank you for your contribution. To get your PR merged there must be some changes made and a unit test be written.

Additionally it is required that your code must be up to date with the main branch. Please do not merge the current code into your branch but rebase your current branch onto main. In this way you are not adding unnecessary merge commits to the history.

If you like to get into the development of GirCore or you have some more questions you are very welcome to join the project chat at: #gircore:matrix.org.

[badcel]

It is not sufficient to call _handle.Dispose() here.

If you take a look at the definition of the open function it looks like:

public static extern GLib.Internal.DirUnownedHandle Open([MarshalAs(UnmanagedType.LPUTF8Str)]string path, uint flags, out GLib.Internal.ErrorOwnedHandle error);

The returned handle is an DirUnownedHandle meaning it does not own the data which is returned by open which in turn means it is not allowed to free the memory. Calling Dispose on the handle will actually do nothing except setting the state of the handle to "Closed" (which is fine).

To actually free the memory you need to take a look into the documentation: https://docs.gtk.org/glib/type_func.Dir.open.html

There you will find this sentence for the return value:

A newly allocated GDir on success, NULL on failure. If non-NULL, you must free the result with g_dir_close() when you are finished with it.

So to actually free the memory the user must call close. To really Dispose the data the dispose method must look like:

public void Dispose()
{
    Internal.Dir.Close(_handle);
    _handle.Dispose();
}

Additionally to ensure the Dispose method is available and does not thrown an exception I would like to have a unit test in src/Tests/Libs/GLib-2.0.Tests/Records/DirTest.cs:

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace GLib.Tests;

[TestClass, TestCategory("IntegrationTest")]
public class DirTest
{
    [TestMethod]
    public void CanDispose()
    {
        var dir = (IDisposable) Dir.Open(".", 0);
        dir.Dispose();
    }
}

[fleimeris]

Thanks for a review. I just have one question, why as IDisposable in test is needed ? Since Dir already uses IDisposable interface.

[badcel]

If the interface would be removed by accident nobody would notice.

I updated the unit test code to avoid a null warning.

[fleimeris]

Okay, that makes sence. I've commited requested changes.

[badcel]

I updated my example for the unit test to a direct cast of the IDisposable interface, can you implement it like that? This avoids a null reference warning in the unit test as the as operator could return null.

var dir = (IDisposable) Dir.Open(".", 0);

Did you see my comment above this conversation? I asked to not merge main into this branch but rebase onto main instead. If I would merge your PR now, there would be 3 commits in the history instead of one. Now as you merged the changes, can you please apply the mentioned change and afterwards squash all your commits into a single one so the git history is kept clean?

[fleimeris]

Okay it worked, thanks.

[badcel]

Great it worked 👍

If you have a Git UI they often support a lot of those features (some more, some less). It is worth to get familiar with git and it's tools as it is a massive time saver to get a clean history.

[fleimeris]

Again, sorry for wasting a time, thought would be simple commit and that's it, but learned a lot, thanks to you.
I'm using Github Desktop, but I released that I need to use git in CLI, because I know nothing about how git works.

[badcel]

I would assume that Github Desktop is not a very bad tool. It supports squashing and amending and more:

• https://docs.github.com/en/desktop/contributing-and-collaborating-using-github-desktop/managing-commits/squashing-commits
• https://docs.github.com/en/desktop/contributing-and-collaborating-using-github-desktop/managing-commits/amending-a-commit

I don't know if using the commandline makes a huge difference. The important thing is to try to understand what is happening in git. Once understood it is quite easy to manage your code. Then you "only" need to master some kind of tool (either the CLI interface or an application) to make git do what you want.

[fleimeris]

I'm not saying that Github Desktop is a bad tool, I think it's a great tool, but at least for me while using Github Desktop, I did not had an incentive to understand what is happening in git until now. But using git in cli will for sure have direct or indirect push for mastering git.

[badcel]

Thank you very much for your contribution 🚀