Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise the code for loading configuration files #1519

Merged

Conversation

masatake
Copy link
Member

@masatake masatake commented Aug 2, 2017

option-directory feature was not available on platforms
where scandir is not available.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling 41460a9 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

main/options.c Outdated
@@ -511,7 +507,7 @@ static struct Feature {
#ifdef DEBUG
{"debug", "TO BE WRITTEN"},
#endif
#ifdef HAVE_SCANDIR
#if defined(HAVE_SCANDIR) || defined (HAVE_DIRENT_H) || defined (_MSVC)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_MSVC should be _MSC_VER.

if (counter + 1 == allocated) {
allocated <<= 1;
array = (struct dirent **)
realloc_safe((char *)array, allocated * sizeof(struct dirent *));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling dc52e6f on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@masatake masatake force-pushed the portable-scandir-implementation branch from dc52e6f to a15c692 Compare August 3, 2017 02:49
@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling a15c692 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling eb720c3 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@masatake masatake force-pushed the portable-scandir-implementation branch from eb720c3 to c9fa895 Compare August 3, 2017 04:32
@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling c9fa895 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@masatake
Copy link
Member Author

masatake commented Aug 3, 2017

O.k. we can use scandir any platform where ctags runs.

I will remove ~/.ctags file support from u-ctags. Instead I will introduce ~/.u-ctags DIRECTORY.
After introducing the directory, I can write ctags-optlib.5.

@k-takata
Copy link
Member

k-takata commented Aug 3, 2017

msbuild fails with unknown error.
https://ci.appveyor.com/project/universalctags/ctags/build/1.0.187/job/2qhhrnjdpapk8xal#L345
It is very strange that ctags --version fails.

msvc x64 build fails with the option-extras-enabling-all test.
https://ci.appveyor.com/project/universalctags/ctags/build/1.0.187/job/t8gh3aic9b11ykxf#L1326

@masatake masatake force-pushed the portable-scandir-implementation branch from c9fa895 to a79f557 Compare August 3, 2017 18:38
@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling a79f557 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling b77c8be on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

#endif

#else
#erorr "No dirent.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still failing because of these two lines. You can remove them now.
If you think they are still needed, then you should move them between L388 and L389.
s/#erorr/#error/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, Thank you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last change is not enough. I meant:

--- a/main/portable-dirent.h
+++ b/main/portable-dirent.h
@@ -382,8 +382,8 @@ static void rewinddir(DIR* dirp)
 }
 #endif
 
-#else
-#error "dirent.h is not available."
 #endif /*DIRENT_H*/
 
+#else
+#error "dirent.h is not available."
 #endif /* HAVE_DIRENT_H */

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling a5a0a29 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@masatake
Copy link
Member Author

masatake commented Aug 4, 2017

@k-takata, I have a question about filename on Windows.

https://ja.wikipedia.org/wiki/8.3%E5%BD%A2%E5%BC%8F (Japanese)

This limitation(?) about file name is still applicable (or existing) in Windows these days?

I would like to use '.ctags' as an extension for ctags option library on all platforms where u-ctags runs.

However, man page of exuberant-ctags says


FILES
       /ctags.cnf (on MSDOS, MSWindows only)
       /etc/ctags.conf
       /usr/local/etc/ctags.conf
       $HOME/.ctags
       $HOME/ctags.cnf (on MSDOS, MSWindows only)
       .ctags
       ctags.cnf (on MSDOS, MSWindows only)

.cnf is used as an extension.

I think we can forget MSDOS. Do you agree this?
If the answer is yes, can we use .ctags intead of .cnf as an extension on MSWindows?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling c9f6e48 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@k-takata
Copy link
Member

k-takata commented Aug 4, 2017

I don't think we should still care about the 8.3 filenames.
We can use .ctags as an extension.
However, we should care about filenames starting with a dot. (E.g. $HOME/.ctags)
Windows filesystems can store such filenames, but need a trick to create them on Explorer (and other GUI tools). If a use create a filename with .ctags. (with an additional dot), it will be stored as .ctags.

@k-takata
Copy link
Member

k-takata commented Aug 4, 2017

Another new errors: https://ci.appveyor.com/project/universalctags/ctags/build/1.0.191/job/mgh91tgiii06simc#L72

main\portable-scandir.c(200) : error C2039: 'd_ino' : is not a member of 'dirent'
        c:\projects\ctags\main\portable-dirent.h(167) : see declaration of 'dirent'
        c:\projects\ctags\main\portable-dirent.h(167) : see declaration of 'dirent'
main\portable-scandir.c(201) : error C2039: 'd_reclen' : is not a member of 'dirent'
        c:\projects\ctags\main\portable-dirent.h(167) : see declaration of 'dirent'
        c:\projects\ctags\main\portable-dirent.h(167) : see declaration of 'dirent'

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling dbde971 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@masatake
Copy link
Member Author

masatake commented Aug 4, 2017

I don't think we should still care about the 8.3 filenames.
We can use .ctags as an extension.
However, we should care about filenames starting with a dot. (E.g. $HOME/.ctags)
Windows filesystems can store such filenames, but need a trick to create them on Explorer (and other GUI tools). If a use create a filename with .ctags. (with an additional dot), it will be stored as .ctags.

Thank you. I hope I can find time to think about the new file names in this weekend.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling 70caa13 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling cb184a3 on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 85.386% when pulling ce12dba on masatake:portable-scandir-implementation into 6ad80f4 on universal-ctags:master.

@@ -0,0 +1,243 @@
#include "general.h"
/*
* Taken from https://github.com/ClusterLabs/pacemaker/master/replace/scandir.c
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it better to use non-raw URL, so that one can easily get the information (history, etc.) of the file?
https://github.com/ClusterLabs/pacemaker/blob/master/replace/scandir.c

* The original author put this code in the public domain.
* It has been modified slightly to get rid of warnings, etc.
*
* Below is the email I received from pinard@iro.umontreal.ca (Fran�ois Pinard)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Encoding seems broken.
s/Fran�ois/François/
The original file was latin 1 (ISO 8859-1), but maybe it's batter to use UTF-8?

*
* Subject: Re: Scandir replacement function
* Date: 18 May 2001 12:00:48 -0400
* From: pinard@iro.umontreal.ca (Fran�ois Pinard)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

@masatake masatake force-pushed the portable-scandir-implementation branch from ce12dba to 44c718c Compare August 4, 2017 04:12
@masatake
Copy link
Member Author

masatake commented Aug 4, 2017

@k-takata, reflected the your comment to the changes and updated.

I update portable-getdents.h. So we will get build-errors again.

@k-takata
Copy link
Member

k-takata commented Aug 4, 2017

eMalloc and eFree are used in portable-dirent.h, but they are declared in routine.h after including portable-dirent.h.
portable-dirent.h should be included in routine.h after declaring them.

Now PATH_MAX is defined in portable-dirent.h, but routines.c also defines PATH_MAX in line 140, so they conflict. Line 140 can be removed.

@masatake
Copy link
Member Author

masatake commented Aug 4, 2017

Something wrong in circleci. However, we can ignore it.

@k-takata, thank you for great advices. Especially the last comment is really helpful.
I have taken too many mistakes. A bit tired.

I will squash some of commits.

@masatake masatake force-pushed the portable-scandir-implementation branch from 281a57a to 1f89dbb Compare August 4, 2017 12:21
…d .d when loading optlib files

TODO

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Spotted by @k-takata.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Does the same as ``--options`` but doesn't make an error if *file* (or
*directory*) doesn't exist.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Suggested by @k-takata.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake masatake force-pushed the portable-scandir-implementation branch from 61ba2cf to 6c25428 Compare October 13, 2017 09:51
@masatake
Copy link
Member Author

Squashed some "fixup!" commits.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 85.943% when pulling 6c25428 on masatake:portable-scandir-implementation into be525b6 on universal-ctags:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants