include: split public and private headers & prototype libsclang interface #703

Merged
merged 10 commits into from Jan 7, 2013

Conversation

Projects
None yet
3 participants
@timblechmann
Contributor

timblechmann commented Dec 31, 2012

hi all,

i'd like to merge this patch. it does:

  • split the headers into public (in include/) and private (in lang/ or server/).
  • clean up the public interface for libsclang (only expose the SC_TerminalClient class)
  • remove some dead code

jakob, two points for you:

  • i know you have been working on a plugin interface for the language. that should be unrelated to the changes of the libsclang interface, as i've only dealt with the interface, how to use libsclang.
  • i'm not sure what's the best way to incorporate QtCollider::LangClient, but maybe we can simply provide a factory function which returns a TerminalClient *, which could be a QtCollider::LangClient instance.

thoughs?

Signed-off-by: Tim Blechmann tim@klingt.org

@jleben

This comment has been minimized.

Show comment
Hide comment
@jleben

jleben Jan 1, 2013

Member

i know you have been working on a plugin interface for the language. that should be unrelated to the changes of the libsclang interface, as i've only dealt with the interface, how to use libsclang.

Not precisely. The plugin interface also modifies the headers affected by this patch, in order to extract a minimal needed subset of API. But that means only an inconvenience and is no big deal to solve, so never mind.

i'm not sure what's the best way to incorporate QtCollider::LangClient, but maybe we can simply provide a factory function which returns a TerminalClient *, which could be a QtCollider::LangClient instance

Sounds good!

Member

jleben commented Jan 1, 2013

i know you have been working on a plugin interface for the language. that should be unrelated to the changes of the libsclang interface, as i've only dealt with the interface, how to use libsclang.

Not precisely. The plugin interface also modifies the headers affected by this patch, in order to extract a minimal needed subset of API. But that means only an inconvenience and is no big deal to solve, so never mind.

i'm not sure what's the best way to incorporate QtCollider::LangClient, but maybe we can simply provide a factory function which returns a TerminalClient *, which could be a QtCollider::LangClient instance

Sounds good!

@kaoskorobase

This comment has been minimized.

Show comment
Hide comment
@kaoskorobase

kaoskorobase Jan 1, 2013

Contributor

fwiw, LanguageClient was meant to be an abstract public interface and should be returned by the factory function, unless it's missing functionality?

Contributor

kaoskorobase commented Jan 1, 2013

fwiw, LanguageClient was meant to be an abstract public interface and should be returned by the factory function, unless it's missing functionality?

@jleben

This comment has been minimized.

Show comment
Hide comment
@jleben

jleben Jan 2, 2013

Member

fwiw, LanguageClient was meant to be an abstract public interface and should be returned by the factory function, unless it's missing functionality?

Both LanguageClient and TerminalClient are useful publicly: the latter has additional public interface to be used externally, and would have no sense as a virtual interface of LanguageClient.

Member

jleben commented Jan 2, 2013

fwiw, LanguageClient was meant to be an abstract public interface and should be returned by the factory function, unless it's missing functionality?

Both LanguageClient and TerminalClient are useful publicly: the latter has additional public interface to be used externally, and would have no sense as a virtual interface of LanguageClient.

@timblechmann

This comment has been minimized.

Show comment
Hide comment
@timblechmann

timblechmann Jan 2, 2013

Contributor

jakob, which parts of SC_TerminalClient do you want to expose publicly? i suppose run or quit would probably make sense? (at least run is used in cmdLineFuncs.cpp)

Contributor

timblechmann commented Jan 2, 2013

jakob, which parts of SC_TerminalClient do you want to expose publicly? i suppose run or quit would probably make sense? (at least run is used in cmdLineFuncs.cpp)

@timblechmann

This comment has been minimized.

Show comment
Hide comment
@timblechmann

timblechmann Jan 2, 2013

Contributor

also, stefan, why did you expose the symbols? afaict the methods using them could be be hidden in the implementation, so that PyrSymbol could be moved completely out of the public interface

Contributor

timblechmann commented Jan 2, 2013

also, stefan, why did you expose the symbols? afaict the methods using them could be be hidden in the implementation, so that PyrSymbol could be moved completely out of the public interface

@kaoskorobase

This comment has been minimized.

Show comment
Hide comment
@kaoskorobase

kaoskorobase Jan 2, 2013

Contributor

it was meant as an optimization for often sent symbols and i think i've used them in a GUI client once. i think it would be better to hide them and keep PyrSymbol out of the public interface as you say. thanks for this patch, it was long needed ;)

Contributor

kaoskorobase commented Jan 2, 2013

it was meant as an optimization for often sent symbols and i think i've used them in a GUI client once. i think it would be better to hide them and keep PyrSymbol out of the public interface as you say. thanks for this patch, it was long needed ;)

include: split public and private headers & prototype libsclang inter…
…face

Signed-off-by: Tim Blechmann <tim@klingt.org>
@timblechmann

This comment has been minimized.

Show comment
Hide comment
@timblechmann

timblechmann Jan 3, 2013

Contributor

looking at the code, i'd suggest to move SC_StringBuffer out of the public interface. one cannot use it without compiling the related source file, but it is trivial to use the LanguageClient without ...

Contributor

timblechmann commented Jan 3, 2013

looking at the code, i'd suggest to move SC_StringBuffer out of the public interface. one cannot use it without compiling the related source file, but it is trivial to use the LanguageClient without ...

@kaoskorobase

This comment has been minimized.

Show comment
Hide comment
@kaoskorobase

kaoskorobase Jan 3, 2013

Contributor

On 03/01/2013, at 10:23, Tim Blechmann notifications@github.com wrote:

looking at the code, i'd suggest to move SC_StringBuffer out of the public interface. one cannot use it without compiling the related source file, but it is trivial to use the LanguageClient without ...

ok, but you would need to link against libsclang anyway so no need to compile the module for users of the library, no? maybe an stl equivalent could be used instead? i wrote the string buffer only because james was reluctant to depend on the stl back then.

sk

Contributor

kaoskorobase commented Jan 3, 2013

On 03/01/2013, at 10:23, Tim Blechmann notifications@github.com wrote:

looking at the code, i'd suggest to move SC_StringBuffer out of the public interface. one cannot use it without compiling the related source file, but it is trivial to use the LanguageClient without ...

ok, but you would need to link against libsclang anyway so no need to compile the module for users of the library, no? maybe an stl equivalent could be used instead? i wrote the string buffer only because james was reluctant to depend on the stl back then.

sk

@timblechmann

This comment has been minimized.

Show comment
Hide comment
@timblechmann

timblechmann Jan 3, 2013

Contributor

linking to the library is one thing, exporting the symbols of the class is another (atm SC_StringBuffer is not exported). but afaict it does not look too different from a std::string or std::vector

Contributor

timblechmann commented Jan 3, 2013

linking to the library is one thing, exporting the symbols of the class is another (atm SC_StringBuffer is not exported). but afaict it does not look too different from a std::string or std::vector

timblechmann added some commits Jan 2, 2013

sclang: SC_LanguageClient - move symbols out of the interface
Signed-off-by: Tim Blechmann <tim@klingt.org>
sclang: terminal client - move PyrSymbol out of the interface
Signed-off-by: Tim Blechmann <tim@klingt.org>
sclang: language client - move private members to hidden class
Signed-off-by: Tim Blechmann <tim@klingt.org>
sclang: use SC_APP preprocessor symbol to configure scapp build
cleans up the initialization code for primitives

Signed-off-by: Tim Blechmann <tim@klingt.org>
sclang: LanguageClient - provide factory function
Signed-off-by: Tim Blechmann <tim@klingt.org>
@timblechmann

This comment has been minimized.

Show comment
Hide comment
@timblechmann

timblechmann Jan 3, 2013

Contributor

ok, after exposing run() i think we can even move the SC_TerminalClient out of the public interface. jakob, i've changed the initialization of qtcollider a bit (moving it to the factory). could you double-check if everything is correct?

Contributor

timblechmann commented Jan 3, 2013

ok, after exposing run() i think we can even move the SC_TerminalClient out of the public interface. jakob, i've changed the initialization of qtcollider a bit (moving it to the factory). could you double-check if everything is correct?

timblechmann added some commits Jan 3, 2013

common: move private headers from include/common to common
Signed-off-by: Tim Blechmann <tim@klingt.org>
plugin interface: move struct SC_Lock out of the public interface
plugins only acquire the NRT lock, but should to that via the interface
table

Signed-off-by: Tim Blechmann <tim@klingt.org>
scapp: add include dirs for private headers
Signed-off-by: Tim Blechmann <tim@klingt.org>

timblechmann added a commit that referenced this pull request Jan 7, 2013

Merge pull request #703 from timblechmann/topic/public_interface
include: split public and private headers & prototype libsclang interface

@timblechmann timblechmann merged commit 64e78cf into supercollider:master Jan 7, 2013

@2mc 2mc referenced this pull request Jan 8, 2013

Merged

out-comment s_tick #709

timblechmann added a commit that referenced this pull request Jan 8, 2013

Merge pull request #709 from 2mc/masters_tick
Merge pull request #703 broke 'no-ide compiles' due to the duplicate symbol 's_tick' introduced by commit 8807e9e. I guess, occurrence of 's_tick' in 'GUIPrimitives.m' is a left-over and should be removed there.

@timblechmann timblechmann deleted the timblechmann:topic/public_interface branch Jul 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment