patch: MC for Win32
Pavel Roskin
proski at gnu.org
Thu Nov 29 02:56:19 UTC 2001
Hi, Franco!
I'm trying to apply your patch (and applying it partially) and here are
some notes.
> - I took a CVS checkout from 27.Okt as a basis
Strange that you are referencing gtkedit in user.h. Well, it's my guilt.
Miguel asked me not to remove the gnome directories from the main branch.
I should have argued that if there are any CVS problems they should be
fixed in CVS, not worked around. Besides, it there are such problems,
it's better if they are easy to notice (the whole directory disappears)
rather than when they are subtle (one file is missing somewhere).
I established an module "mc-text" on CVS but you were obviously using the
complete checkout. Sorry for inconvenience :-(
I'm fixing this error. The unused files have been removed from the head
branch. Now it's safe again to use "cvs update -d" without risking to get
GNOME-related files.
> You can build mc.exe by changing to the pc subdir and run
> $> make -fMakefile.MIN
Please leave a space in all documentation (FAQ etc) as it's safer and less
confusing.
> I added the two files
>
> mc/pc/regex.c
> mc/pc/regex.h
I've committed them. We'll remove them later and a few lines in ChangeLog
won't increase the size of the distribution too much :-)
> mc/edit/edit.c
> - Open file in binary mode on Win32 platforms.
Please don't break things too much. It should apply to OS/2 as well.
Then why _OS_NT and not OS2_NT? If your idea is wrong, then OS/2 porters
will fix it for you, but let's not require them to do it everywhere.
Besides, if you expect this fix to be used in many places, it's better to
define something as "b" and use it instead of ifdefs:
#ifdef OS2_NT
#define FOPEN_BINARY "b"
#else
#define FOPEN_BINARY
#endif
fopen(file, "r" FOPEN_BINARY)
> mc/pc/Makefile.MIN
> - removed ".exe" from mingw-binary names
> to make build possible on cross-compilers
Does it work in Windows?
> - fixed compiler and linker flags for use of glib and gettext
> - fixed order of flags for linking mc.exe
Fine, but please avoid double spaces - nobody needs them.
> mc/pc/Makefile.PC
> - added/removed files to/from build
You don't update files for OS/2. It's easier that you keep things in
sync, or somebody will think whether there is a reason for that
difference.
> mc/pc/chmod.c
> - fixed includes for cross-compiler
> - added gettext N_() and _() makro calls
Fine. Committed.
> mc/pc/config.h
> - a few fixes
I don't understand some of them. Perhaps backslash in includes should be
elininated globally. Everything else requires explanation.
> mc/pc/cons_nt.c
> - changes regarding the calculation of the size of the console window on Win32
> fixes a bug with Win2k
I have Win2000 now. I'll apply it when I check it.
> mc/pc/dirent.h
> - added comment that file seems to be no longer needed
It's not just a comment. Be honest. I read your patch. If the file is
not needed - remove it.
> mc/pc/drive.c
> - added gettext N_() and _() makro calls
> - added workaround for a crash that happens
> when Changing drive while panel not in DirListing mode
The gettext part is fine. Please use better coding style for the new
code.
> mc/pc/key_nt.c
> - fixed a problem with AltGr Key
> - added/changed dummy functions (mouse dummys)
The same comment applies. Please never put "return" to the same line as
"if" since it's hard to debug. Not to mention whole functions.
> mc/pc/slint_pc.c
> - added comment -> DO NOT USE GETTEXT HERE
Not very useful comment. There are documented means for that
(POTFILES.ignore if I remember correctly).
> mc/pc/util_nt.c
> - fixed includes
> - added gettext N_() and _() makro calls
> - fixed problem with the display of free disc space
> - added dummy functions
Cannot you just use global.h - it's supposed to take care of includes?
Please avoid C++ comments in C programs.
> mc/slang/slerr.c
> - fixed includes
Why is this fix needed? Why is it Win32-specific? I would think that you
should rather fix something else. Same applies to all slang files.
> mc/src/cmd.c
> - fixed includes
How about global.h? Doesn't it already include sys/time.h?
It it posible to add the top-level directory to the includes so that
"vfs/vfs.h" works?
> mc/src/command.c
> - added handling of internal SET command for Win32
I don't like that you are mixing new features and pure build fixes. But
how about converting the command to lowercase. Otherwise "Cd" is not
supported, but "Set" is.
> mc/src/ext.c
> - no shellbang in temporary command file on windows
Will apply.
> mc/src/file.c
> - fixed includes
> - commented out the detection of cyclic symlinks on Win32
> this is only a Quick Fix
Will apply the second part. I don't like your approach to fixing the
includes.
> mc/src/global.h
> - fixed includes
What's wrong with utime.h? Why don't you want to include it even if
HAVE_UTIME_H is defined?
> mc/src/main.c
> - fixed includes
> - director_history_add is broken on Win32 - mc crashes -
> made it a dummy function on Win32
> THIS definately has to be fixed
> - made init_xterm_support a dummy on Win32 - there is no xterm there
> - made mc ignore Ctrl-C breaks on Win32
> - defined the gettext LOCALEDIR for Win32
> - added comment that gfree(home_dir); might no longer be needed on Win32
Please don't include local files (in quotes) before system includes.
> mc/src/myslang.h
> - fixed includes
Again, you must be doing something wrong.
> mc/src/treestore.c
> - commented out
> process_special_dirs (&special_dirs, CONFDIR "mc.global");
> for Win32
Why?
> mc/src/user.h
> - fixed includes
Again: there is no such thing as gtkedit in the CVS MC.
> mc/src/util.c
> - fixed includes
> - fixed parameters for bizp2 might be also needed for other platforms
Will apply after testing.
> - fixed TEMPDIR for Win32
What was wrong with it?
> mc/vfs/vfs.h
> - fixed includes
Again, what's wrong with utime.h? If there are any problems with it it's
better to use it in global.h only and move all tricks there.
--
Regards,
Pavel Roskin
More information about the mc-devel
mailing list