getopt.h / wgetopt.h cleanup #2790
Comments
|
I can't find any reason why a workaround for a missing getopt_long() function was needed in early 2006. Googling shows it had been part of the GNU library for several years by then. I can only surmise that some of the mainstream UNIX distros like Solaris may not have had an implementation at that time. But since BSD, OS X, Linux, Solaris all have it now there is no good reason to keep that baggage. For the record the workaround for a missing getopt_long() function was introduced by ~axel on 2006-08-28 via commit 548e379. It was to avoid doing the same check in fishd.c, mimedb.c and set_color.c none of which still exist. All of the changes to those lines of code since then only make non-functional changes (e.g., adding comments, lint removal). The aforementioned workaround in fallback.h was introduced after the initial introduction of getopt_long() use guarded by |
|
I also noticed, while addressing the original issue, that there is inconsistency in how the fish private wgetopt_long() function is used. Not to mention silly things like conflating the NULL pointer and zero (yeah, I know that isn't a problem in practice, but it's still ugly). |
|
I'm not going to convert fish.cpp and fish_indent.cpp to use wgetopt_long at this time. I (or someone else) needs to first ensure that such changes are compatible with locale settings that are not UTF-8. |
|
Sounds like a good idea. |
|
For those (like me) unfamiliar with Solaris, version 9 achieved EOL in October 2014, version 10 was released in January 2005. |
There is no longer a good reason to detect whether or not getopt_long() is available. All UNIX implementations we're likely to run on have it. And if we ever find one that doesn't the right thing to do is not fallback to getopt() but to include the getopt_long() source in our package like we do with the pcre2 library. Since it's licensed under LGPL we can legally do so if it becomes necessary. This partially addresses issue #2790.
There is no longer a good reason to detect whether or not getopt_long() is available. All UNIX implementations we're likely to run on have it. And if we ever find one that doesn't the right thing to do is not fallback to getopt() but to include the getopt_long() source in our package like we do with the pcre2 library. Since it's licensed under LGPL we can legally do so if it becomes necessary. This partially addresses issue fish-shell#2790.
There is no longer a good reason to detect whether or not getopt_long() is available. All UNIX implementations we're likely to run on have it. And if we ever find one that doesn't the right thing to do is not fallback to getopt() but to include the getopt_long() source in our package like we do with the pcre2 library. Since it's licensed under LGPL we can legally do so if it becomes necessary. This partially addresses issue fish-shell#2790.
While working on issue #102 I noticed that src/fallback.h defines several values normally found in the systems getopt.h header. Which wouldn't be an issue except that it defines them piecemeal which means they can easily conflict with symbols defined before src/fallback.h is included. In practice that is extremely unlikely but is still a potential problem. What's worse, however, is the code uses fish's wgetopt implementation everywhere but fish.cpp, fish_indent.cpp, and fish_tests.cpp. It seems to me that those getopt symbols should be removed from fallback.h and those modules should be using fish's wgetopt implementation.
P.S., None of the existing labels are appropriate. I'd select something like "lint" or "cleanup" if they existed.
The text was updated successfully, but these errors were encountered: