I've been running the ever so slightly different implementation of this for a
long while and it worked nicely for me. Just tested this PR again, and it
seems to indeed actually behave the same, so I'm merging it.
--
Reply to this email directly or view it on GitHub:
Merged #3490 into master.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3490#event-10582215719
You are receiving this because you are subscribed to this thread.
Message ID:
@b4n I've just repushed a hybrid version of the patch - using the
`trust_file_scope` name but using `g_str_has_suffix()` as I used in the
original patch which I like a bit better (but I don't insist on that one too
much if the other version is preferred).
Now the hard stuff - someone has to
@techee pushed 2 commits.
db5d7e5cac00de7f9e2aee48b05bde5397370dd2 Add a flag indicating whether to
trust isFileScope from ctags
2fd242b94c41f9254b2288fdb39a1c20d819b53d For C/C++ only mark tag as local if
it originates from a source file
--
View it on GitHub:
> I'll look a tad into modules out of curiosity one day, but does that mean
> that a kind of header equivalent is generated by the compiler, so that it can
> be installed and imported?
Presumably somewhere internal to compiler data files somewhere I guess ...
oooh, that google is good
> So if we set the filescope/local/whatever flag for `.cpp` files that will
> make the symbols exported with the module invisible elsewhere, but they
> should be visible.
Just looking quickly at https://en.cppreference.com/w/cpp/language/modules
suggests that there will be an `export` keyword,
> Just to flesh out how C++ modules impact this. […]
I'll look a tad into modules out of curiosity one day, but does that mean that
a kind of header equivalent is generated by the compiler, so that it can be
installed and imported?
--
Reply to this email directly or view it on GitHub:
I've been running this (well, almost… see below :grin:) for a couple days and
it seems to work nicely
My "small" patch on top just because:
```diff
diff --git a/src/tagmanager/tm_ctags.c b/src/tagmanager/tm_ctags.c
index a5cb9531e..16e2ebf04 100644
--- a/src/tagmanager/tm_ctags.c
+++
Just to flesh out how C++ modules impact this.
Modules break the assumption that there must be a declaration in a header file
for a symbol to be used in another file, the module is exported from a body
file and the declarations from the modules are imported by an `import`
statement, no
> only for C and C++ ATM
And in a heartbeat he drops the obligatory "C++ isn't C" comment.
Although not used much yet, modules have been a thing since C++20, three years
... and the interfaces are in the `.cpp` files not in "header" files.
Not to mention private modules.
And declarations
@elextr well, we do that only for C and C++ ATM (see code), but that's
basically the reason why I suggest a generic name merely stating how the flag
is used rather than what we try to achieve with it (which does not make sense
for languages with no header/source separation)
--
Reply to this
> only if it matches a known non-header extension.
Won't that break a whole lot of languages where there is no separation of
header and body, where modules from one body file are imported into another
body file?
--
Reply to this email directly or view it on GitHub:
@elextr no, that's what I though initially, but it's actually because of the
`isFileScope` filtering. uctags sets it when it does *not* match a known header
extension, which my file didn't. Here we're just taking the more conservative
approach of setting it only if it *matches* a known
> Actually if we want that to work for all languages, we could make it
> technical: trust_file_scope (which is really what this flags is about:
> whether we want to trust uctags' isFileScope)
Isn't the real question if the Geany flag is a copy of isFileScope or our own
heuristic, or some
> Based on my following post
> `semi_random_guess_if_symbols_should_leak_everywhere` grin
>
> Hmmm, maybe just simply `globally_visible`?
>
> [Edit: oops, wrong way round, maybe `local_visibility`]
Actually if we want that to work for all languages, we could make it technical:
> You could come up with a better name for the flag
Yeah, I only realised that the problem was the name of the flag after I posted
that.
Based on my following post
`semi_random_guess_if_symbols_should_leak_everywhere` :grin:
Hmmm, maybe just simply `globaly_visible`?
> But I agree with
@elextr interesting… I usually see this still using separate source compiled
conditionally with a common header defining the interface. But OK, so you have
a case where there is no way to know but having complete knowledge of the whole
project tree -- which we won't have (well, a plugin like
> What about header files, why are they not C/C++ source?
You could come up with a better name for the flag, maybe
`most_probably_not_a_file_whose_local_symbols_will_be_visible_to_others`?
:grin:
--
Reply to this email directly or view it on GitHub:
> Do you have some concrete examples of when including source files is used?
An example in one of the projects I'm currently doing has platform specific
files included like:
```cpp
#if defined(LIN)
#include "plat_lin.cpp"
#elif defined(WIN)
#include "plat_win.cpp"
#elif defined(MAC)
#include
@b4n commented on this pull request.
> + if (source_file->lang == TM_PARSER_C || source_file->lang ==
> TM_PARSER_CPP)
+ {
+ const gchar **ext;
+ const gchar *common_src_exts[] =
+ {".c", ".C", ".cc", ".cp", ".cpp", ".cxx", ".c++",
@techee commented on this pull request.
> + if (source_file->lang == TM_PARSER_C || source_file->lang ==
> TM_PARSER_CPP)
+ {
+ const gchar **ext;
+ const gchar *common_src_exts[] =
+ {".c", ".C", ".cc", ".cp", ".cpp", ".cxx", ".c++",
> Sometimes we include C files in other C files. Then static is not file-local.
> So we should probably also consider includes (not just files with header
> extension) or just revert the original change.
That's true, I'm just wondering how such files typically look like and for what
purpose
Since (IIUC) the cause of #3454 is the intention to limit the symbols visible
at any point by using heuristics (originally dodgy uctags ones) to exclude
symbols that definitely can't be visible. The "its not the same language so it
can't be visible" heuristic is the cause of the OP of #3454
Sometimes we include C files in other C files. Then `static` is not
file-local. So we should probably also consider includes (not just files with
header extension) or just revert the original change.
--
Reply to this email directly or view it on GitHub:
What about header files, why are they not C/C++ source?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3490#issuecomment-1539820100
You are receiving this because you are subscribed to this thread.
Message ID:
@b4n commented on this pull request.
> + if (source_file->lang == TM_PARSER_C || source_file->lang ==
> TM_PARSER_CPP)
+ {
+ const gchar **ext;
+ const gchar *common_src_exts[] =
+ {".c", ".C", ".cc", ".cp", ".cpp", ".cxx", ".c++",
I'll try to test it properly tomorrow, but the approach, although a bit of a
workaround (as it's basically overriding a similar check in uctags), it looks
pretty good indeed (esp. with the discussion on the uctags PR).
--
Reply to this email directly or view it on GitHub:
27 matches
Mail list logo
