habibalamin (حبيب الامين)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 11 2017, 4:53 PM (97 w, 4 d)

Recent Activity

Aug 18 2017

habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

Yes, looks like Phyx just set it ~4 hours ago.

Aug 18 2017, 12:11 PM

Aug 17 2017

habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

Can we add this to the 8.4.1 milestone?

Aug 17 2017, 5:25 PM

Jul 30 2017

habibalamin added a comment to D3726: Allow Windows to set blank environment variables.
In D3726#107429, @Phyx wrote:

I don't understand, Windows API Unicode functions are UTF-16 always and always take a widechar. The W in the name is for Wide after all. Yes UTF-16 and wchar_t are not the same thing, but that doesn't change the fact that the Wide APIs on Windows accept only UTF-16 https://msdn.microsoft.com/en-us/library/windows/desktop/dd374081(v=vs.85).aspx and the article doesn't contradict this.

Jul 30 2017, 4:00 PM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Remove redundant import for Windows.

Jul 30 2017, 11:25 AM

Jul 29 2017

habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

If anyone does pick up the task of writing it in C, this might prove useful - http://utf8everywhere.org. I was under the mistaken assumption that the Windows API's Unicode functions were the ‘right’ ones to use for dealing with Unicode strings in C on Windows.

Jul 29 2017, 3:35 PM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Wrap lines longer than 80 columns.

Jul 29 2017, 3:04 PM
habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

@Phyx won't the title form part of the commit message also? Is a summary necessary if the title is self-explanatory (this ticket has no summary currently)?

Jul 29 2017, 3:04 PM
habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

@Phyx yes, it's the one that was in this diff. I've removed it, I'm giving up. Sorry, I should have made it clear in the diff update message that I was moving on. I've just decided to use the Haskell implementation. @bgamari and I had a conversation about this and leant towards implementing in C for performance reasons (me assuming it would be less difficult than it's proven); in C, GetEnvironmentStrings returns a pointer to the environment variable block, and we can copy the relevant value, free the block and just marshal the single value, whereas in Haskell, we'd have to marshal the whole block of variables through to Haskell-land just to lookup one variable.

Jul 29 2017, 3:04 PM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Remove redundant imports from non-Windows OSes.

Jul 29 2017, 2:39 PM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

I'm having a lot of trouble getting getenv_windows to work in C; I even
tried porting Matz' Ruby Implementation code and I just can't get it to
work. Memory access errors every time.

Jul 29 2017, 2:17 PM

Jul 28 2017

habibalamin added a comment to D3726: Allow Windows to set blank environment variables.
In D3726#107177, @Phyx wrote:

This error happens because you're in an hsc file.

Jul 28 2017, 12:40 PM
habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

I need to head off to work, but I'll leave you with a screenshot of where I'm at. The error in the screenshot is a trivial one, easy to fix; just a basic type mismatch. I'll have to fix it when I get home, as I don't want to start a build right now.

Jul 28 2017, 1:51 AM

Jul 26 2017

habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

Sorry, haven't had a chance to look at this yet. I'll have a couple of hours to look at it tonight after work

Jul 26 2017, 2:36 AM

Jul 22 2017

habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

Damn. I was having this failure (not recognising WINDOWS_CCONV) on my local Windows machine. I still don't know why this happens, but it was initially a local-only problem (masking other errors that Harbormaster showed). I'm gonna try a rebuild, and I'm gonna set up my Windows VM again to debug this tomorrow.

Jul 22 2017, 4:25 PM

Jul 19 2017

habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Initialize value to NULL at start to fix -Werror=maybe-uninitialized failure.

Jul 19 2017, 5:41 PM

Jul 18 2017

habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Fix missing semicolon.

Jul 18 2017, 10:19 PM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Use non-deprecated string functions.

Jul 18 2017, 9:22 PM
habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

I'll probably get on the latest requested changes later today or tomorrow (~10pm here). However, I think we're close.

Jul 18 2017, 3:41 PM
habibalamin added a comment to D3726: Allow Windows to set blank environment variables.
In D3726#106255, @Phyx wrote:

I was looking at https://msdn.microsoft.com/en-us/library/windows/desktop/bb759845(v=vs.85).aspx but a closer look seems to indicate that only the list of functions on that page is deprecated? Looking at the header I don't fully understand why harbormaster is failing. I assume you're testing on x86_64 Windows as well locally?

Jul 18 2017, 3:39 PM
habibalamin added inline comments to D3726: Allow Windows to set blank environment variables.
Jul 18 2017, 2:40 PM
habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

What the hell? Looks like some previous comments were being held and had never posted at all. Man, Phabricator is confusing as hell.

Jul 18 2017, 2:37 PM
habibalamin added a comment to D3726: Allow Windows to set blank environment variables.

Wow, I don't know how I missed that. Where did you find that, though? Because I'm looking at this page: https://msdn.microsoft.com/en-us/library/windows/desktop/bb759954(v=vs.85).aspx, and I don't see anything mentioned about deprecation. I want to look at the documentation, so I can find the modern equivalents.

Jul 18 2017, 2:36 PM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Add GHC Trac ticket reference to release notes.

Jul 18 2017, 10:54 AM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Include Windows.h, which defines LPSTR and other typedefs.

Jul 18 2017, 10:28 AM

Jul 17 2017

habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Add shlwapi to base.cabal extra-libraries.

Jul 17 2017, 4:35 PM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Link Shlwapi.lib file with pragma macro.

Jul 17 2017, 3:23 PM

Jul 16 2017

habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Add changelog entry.

Jul 16 2017, 10:51 AM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Add TODO for macro block defining WINDOWS_CCONV. Fix copyright year.

Jul 16 2017, 10:45 AM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Incorporate review suggestions from Phyx.

Jul 16 2017, 10:42 AM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Fix typos (missing semicolon, slightly wrong function name, etc).

Jul 16 2017, 5:15 AM

Jul 14 2017

habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Fix incompatible pointer type errors by using Microsoft safe string functions.

Jul 14 2017, 12:54 AM

Jul 12 2017

habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Rebase origin/master.

Jul 12 2017, 10:30 AM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Fix Environment.lookupEnv again.

Jul 12 2017, 7:54 AM
habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Fix typo causing incorrect pointer type warnings.

Jul 12 2017, 7:29 AM

Jul 11 2017

habibalamin updated the diff for D3726: Allow Windows to set blank environment variables.

Add getenv for Windows.

Jul 11 2017, 8:02 PM
Herald added a reviewer for D3726: Allow Windows to set blank environment variables: austin.
Jul 11 2017, 5:15 PM