> @elextr I do not understand 'This change will also fail to de-dup paths that > are linked to the same file' I thought, the old code did not do that either, but just hashed it into the same bucket.
Yeah, it looks like that, which seems absurd. My guess would be that the goal was to do what @elextr expects, that is using the inode itself as a key, which indeed would have the nice property of avoid re-computing the same file even under a different path, but as is it actually leads to *never* match any entry (as `g_direct_equal()` on a different string pointer will always deem those different). So your changes are better than the previous behavior, as at least identical paths will match. However, this function seems a bit absurd. I don't see why it doesn't simply builds the list directly, because traversing a GHashTable is not very fast, and IIUC it wouldn't change anything. So we could probably change it to just do that, which would be simpler and fix everyone's concerns. Also, relying on the order of traversal of a GHashTable seems fragile, I don't think it is guaranteed to be stable across GLib versions, architectures or whatnot. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1989#issuecomment-436727913
