SmartIndent refactoring question

[Alpha]

I noticed the recent addition of SmartIndent to the trunk (nice work , and was wondering, is there reasoning for the addition of cbSmartIndentPlugin::InComment() when cbStyledTextCtrl::IsComment() already exists?  The latter function determines what is a comment by using the values defined in the lexer_*.xml files.

It appears selection brace completion no longer works (under Ubuntu).

Also, the unix virtual target "All" does not actually contain all targets.

[oBFusCATed]

It appears selection brace completion no longer works (under Ubuntu).

What do you mean by this? Example steps to reproduce?

[Alpha]

Enable "Selection brace completion" under general editor settings.
Open a C++ source file.
Select a some text.
Type: "("

Select "line"

Code

test line

After typing "(", the expected output (with selection brace completion) is:

Code

test (line)

The actual output is:

Code

test (

[Alpha]

Oh, this is why; the activation logic has changed.

Code

Index: src/plugins/contrib/SmartIndent/CppSmartIndent.cpp
===================================================================
--- src/plugins/contrib/SmartIndent/CppSmartIndent.cpp	(revision 8428)
+++ src/plugins/contrib/SmartIndent/CppSmartIndent.cpp	(working copy)
@@ -74,7 +74,7 @@
 
     const wxChar ch = event.GetKey();
 
-    if ( SelectionBraceCompletionEnabled() )
+    if ( SelectionBraceCompletionEnabled() || stc->IsBraceShortcutActive() ) // the shortcut is Alt-I
         DoSelectionBraceCompletion(stc, ch);
 
     DoSmartIndent(ed, ch);
@@ -464,7 +464,7 @@
 
 void CppSmartIndent::DoSelectionBraceCompletion(cbStyledTextCtrl* control, const wxChar &ch)const
 {
-    if (( control->IsBraceShortcutActive()) && !control->GetLastSelectedText().IsEmpty())
+    if (!control->GetLastSelectedText().IsEmpty())
     {
 
         const int pos = control->GetCurrentPos();

[oBFusCATed]

Okay...

@devs: Shouldn't the options be separated, too? As far as I can see they are still at the old place.

@admins: please move the posts in the proper topic...

[MortenMacFly]

Oh, this is why; the activation logic has changed.

Oh - good catch. Maybe danselmi can check and apply... I'll try myself.

@devs: Shouldn't the options be separated, too? As far as I can see they are still at the old place.

Probably. I think its a matter of taste.

@admins: please move the posts in the proper topic...

To what? I think "Development" fits quite well...

[oBFusCATed]

Probably. I think its a matter of taste.

What happens if the plugins is not loaded?

To what? I think "Development" fits quite well...

I forgot that the other topic is not visible

[danselmi]

...is there reasoning for the addition of cbSmartIndentPlugin::InComment() when cbStyledTextCtrl::IsComment() already exists?  The latter function determines what is a comment by using the values defined in the lexer_*.xml files.

I will look into it. Give me some time.

It appears selection brace completion no longer works (under Ubuntu).

Fixed in 8429.

@devs: Shouldn't the options be separated, too? As far as I can see they are still at the old place.

I already thought about this problem. Which plugin should provide these options? Every one its own dialog with near the same choices (Cpp, fortran, python...)?

[oBFusCATed]

I already thought about this problem. Which plugin should provide these options? Every one its own dialog with near the same choices (Cpp, fortran, python...)?

What about adding a Settings->Editor->Smart Indent page. Then at the top a drop-down would be used to switch between all the implementations.
The implementations can provide their own gui, when they are selected in the drop-down.

[oBFusCATed]

Code

Invalid manifest file for: LuaSmartIndent
Invalid manifest file for: CppSmartIndent

Anyone seeing this in the log? And both Lua and Cpp plugins are not loaded, nor are visible in the Plugins->Manage... dialog.

This is on linux using the autotools build system, rev 8500, but I'm seeing this for a while now.

p.s. can someone rename the plugins to SmartIndentXXX instead of XXXSmartIndent

[Jenna]

Code

Invalid manifest file for: LuaSmartIndent
Invalid manifest file for: CppSmartIndent

Anyone seeing this in the log? And both Lua and Cpp plugins are not loaded, nor are visible in the Plugins->Manage... dialog.

This is on linux using the autotools build system, rev 8500, but I'm seeing this for a while now.

p.s. can someone rename the plugins to SmartIndentXXX instead of XXXSmartIndent

Works fine here with the build from my fedora-repo (8500).
It was build with autotools in a clean environment (mock).

[MortenMacFly]

Works fine here with the build from my fedora-repo (8500).

On Windows here, too. Just tried - but I did a completely clean build, too. Are these files present at all in you copy? Maybe they could not be copied (remember: in the build process they are all being copied from "luamanifest.xml" to "manifest.xml" or alike... I don't recall exactly...

[oBFusCATed]

Last time I've checked there was some mess happening. I'll check again.
The build is 100% clean, because it is done by emerge/portage in a new directory, should be 100% reliable.

[killerbot]

I think I reported this some weeks ago, this pops up all the time, I think the only time (and not even sure) I got rid of this, was a full bootstrap - configure - make process. (On linux). Since then I have switched to ignore phase :-(

[oBFusCATed]

I've removed the svn checkout for emerge and it gone away... pretty strange...

But still someone should rename the plugins to be a more closer together in the UI.

[MortenMacFly]

But still someone should rename the plugins to be a more closer together in the UI.

I'll take the action. Please devs: Do not touch SmartIndent for the moment therefore...

[MortenMacFly]

I'll take the action. Please devs: Do not touch SmartIndent for the moment therefore...

...should be done now.

[Jenna]

I'll take the action. Please devs: Do not touch SmartIndent for the moment therefore...

...should be done now.

Seems to work with automake-system on linux (at least all plugins are loaded).
I committed tiny fixes to debian build-files and added the python smartindent-plugin to the contrib-plugins list in spec-file for rpm and debian/control .

[MortenMacFly]

I committed tiny fixes to debian build-files and added the python smartindent-plugin to the contrib-plugins list in spec-file for rpm and debian/control .

OK - thanks. BTW: I realised SmartIndent is not part of the wx29 build yet.

[killerbot]

today : svn up + make clean + make + sudo make install

It even got worse :

Code

Tools Plus Plugin: Registering shell type Piped Process Control
Plugin manifest could not be parsed: /usr/local/share/codeblocks/FortranSmartIndent.zip
Plugin manifest could not be parsed: /usr/local/share/codeblocks/SmartIndentCpp.zip
Invalid manifest file for: HDLSmartIndent
Invalid manifest file for: SmartIndentXML
Invalid manifest file for: PascalSmartIndent
Plugin manifest could not be parsed: /usr/local/share/codeblocks/SmartIndentHDL.zip
Plugin manifest could not be parsed: /usr/local/share/codeblocks/SmartIndentPython.zip
Invalid manifest file for: LuaSmartIndent
Invalid manifest file for: PythonSmartIndent
Plugin manifest could not be parsed: /usr/local/share/codeblocks/SmartIndentPascal.zip

[MortenMacFly]

today : svn up + make clean + make + sudo make install

There you go: Your error is that the autofoo build system has changed. So you need to make a full bootstrap session, or at least a make dist clean.

(Says the Linux newbie... )

[Jenna]

I suggest not to build C::B from the sources root, but from a seperate build-folder.
That makes it easy to clean everything, if there are too many changes or the build-system is broken for other reasons.

Just a rm -rf * from inside the build-folder and everything is really clean.

[killerbot]

okay, so I did :
- make distclean
- ./bootstrap
- ./configure --with-contrib-plugins=all
- make -j12
- sudo make install


==> does not make any difference

Code

Loaded 0 plugins
Scanning for plugins in /usr/local/lib64/codeblocks/plugins
Tools Plus Plugin: Registering shell type Piped Process Control
Plugin manifest could not be parsed: /usr/local/share/codeblocks/FortranSmartIndent.zip
Invalid manifest file for: SmartIndentCpp
Invalid manifest file for: HDLSmartIndent
Invalid manifest file for: PascalSmartIndent
Plugin manifest could not be parsed: /usr/local/share/codeblocks/SmartIndentHDL.zip
Invalid manifest file for: SmartIndentPython
Invalid manifest file for: LuaSmartIndent
Invalid manifest file for: PythonSmartIndent
Plugin manifest could not be parsed: /usr/local/share/codeblocks/SmartIndentPascal.zip
Invalid manifest file for: SmartIndentFortran
Loaded 59 plugins

Darn autof***  :-(

[killerbot]

I suggest not to build C::B from the sources root, but from a seperate build-folder.
That makes it easy to clean everything, if there are too many changes or the build-system is broken for other reasons.

Just a rm -rf * from inside the build-folder and everything is really clean.

could you elaborate on this ?
with autotools build in a different folder ?

[killerbot]

while I had "/usr/local/lib64/codeblocks/plugins/" open in dolphin,

I did ==> sudo make uninstall

==> all disappeared except :

Code

-rwxr-xr-x 1 root root  1613 Nov  5 07:31 libCppSmartIndent.la
-rwxr-xr-x 1 root root 63650 Nov  5 07:31 libCppSmartIndent.so
-rwxr-xr-x 1 root root  1637 Nov  5 07:31 libFortranSmartIndent.la
-rwxr-xr-x 1 root root 38766 Nov  5 07:31 libFortranSmartIndent.so
-rwxr-xr-x 1 root root  1613 Nov  5 07:31 libHDLSmartIndent.la
-rwxr-xr-x 1 root root 47849 Nov  5 07:31 libHDLSmartIndent.so
-rwxr-xr-x 1 root root  1613 Nov  5 07:31 libLuaSmartIndent.la
-rwxr-xr-x 1 root root 38317 Nov  5 07:31 libLuaSmartIndent.so
-rwxr-xr-x 1 root root  1631 Nov  5 07:31 libPascalSmartIndent.la
-rwxr-xr-x 1 root root 39042 Nov  5 07:31 libPascalSmartIndent.so
-rwxr-xr-x 1 root root  1631 Nov  5 07:31 libPythonSmartIndent.la
-rwxr-xr-x 1 root root 38167 Nov  5 07:31 libPythonSmartIndent.so
-rwxr-xr-x 1 root root  1613 Nov  5 07:31 libXMLSmartIndent.la
-rwxr-xr-x 1 root root 43792 Nov  5 07:31 libXMLSmartIndent.so

I think there's something seriously wrong with out build scripts / makefiles


EDIT : new approach starting with
- rm -rf * in trunk and svn up
- usr/local/share/codeblocks ==> manualy deleted
- usr/local/include/codeblocks ==> manualy deleted
- usr/local/lib64/codeblocks ==> manualy deleted
and then the entire chain again ......... [will paste the result .....]

FAILURE :

Code

zip -R /home/killerbot/CodeBlocks/trunk/src/plugins/contrib/SmartIndent/SmartIndentPython.zip manifest.xml
PWD=`pwd` cd . && \
cp manifest_lua.xml ./manifest.xml && \
zip -R /home/killerbot/CodeBlocks/trunk/src/plugins/contrib/SmartIndent/SmartIndentLua.zip manifest.xml
cp: cannot create regular file `./manifest.xml': File exists
  adding: manifest.xml (deflated 50%)
  adding: manifest.xmlmake[4]: *** [SmartIndentHDL.zip] Error 1
        zip warning:  file size changed while zipping manifest.xml
  adding: manifest.xml (stored 0%)
 (deflated 50%)

damn damn damn

EDIT :
issued again :  make -j12  ==> continues till the end

then issued : sudo make install :  OK

and then we run it and we get (to be honest as expected  :-(   ) :

Code

Loaded 0 plugins
Scanning for plugins in /usr/local/lib64/codeblocks/plugins
Tools Plus Plugin: Registering shell type Piped Process Control
Invalid manifest file for: SmartIndentCpp
Invalid manifest file for: SmartIndentHDL
Invalid manifest file for: SmartIndentPython
Invalid manifest file for: SmartIndentPascal
Loaded 55 plugins

There's something wrong with those SmartIndent, and it seems also not save to touch that area with a make -j12 ??
Never had any build problems before, but it seems SmartIndent is causing problems ...

[oBFusCATed]

1. Why don't you use packages? Provided by Jens or self build
2. Why don't you use prefix (--prefix=/home/myname/software/codeblocks for example), make uninstall is known to be unreliable, especially when there are changes to the filenames
3. For cleaner builds you can use something like:

Code

./bootstrap
cd build
rm -rf *
`pwd`/../configure
make -jn
make install

Most of the time I stick to 1 and I'm happy...

This time I suspect there is some misery happening in the Makefiles, which is not 100% reliable.
What happens if you use make -j1?

[killerbot]

packages why ? I selfbuild CB like this for over then 7 years on linux, never any problems, and very often on more then 5-6 machines.

Why is the make uninstall not reliable ? Is it due to errors in the makefiles, could we correct it ?

Well I have done the cleanup myself as described, will do such cleanup again, and do a build with just 1 core, just to see what happens.
But it is only this SmartIndent which might have a problem, cause why would one not use the multiple cores (those 1 core days are over for me ;-)  )

I will come back with the 1 core result.

EDIT :  the one core build (aka , just : make) ==> OK, so this seems to indicate that the zipping process (??) is probably having a problem during a parallel build. Things still is, never saw any problem with the other plug-ins like that.

[oBFusCATed]

packages why ? I selfbuild CB like this for over then 7 years on linux, never any problems, and very often on more then 5-6 machines.

Your memory is pretty short, but the forum never forgets
Search the forum for all the topics you've started reporting there is a build problem and it turned out you've not used the correct commands.
I doubt the number will be less than 10 

Why is the make uninstall not reliable ? Is it due to errors in the makefiles, could we correct it ?

Because it you do "svn update; make uninstall" it will break if there are changes to the file names
But you can check it yourself by searching you install dir for cb files after you've done a make uninstall

... cause why would one not use the multiple cores (those 1 core days are over for me ;-)  )

Because there might be parallel make issues...

[killerbot]

you are correct I have run into issues, but that was very often pch problems (which seem to be the very nature of pch builds)  
and probably a few I don't remember, I can't even remember which wrong commands I used (could you please point them out to me again, would be good to have my mind refreshed    )


But never such problem where actually the build process itself is screwing up things (like those manifest files / zip files getting broken (?) for SmartIndent with parallel build).

Correct that the uninstall, should happen before the svn up, or we should go for more "forced cleans", eg why clean fopr example /usr/local/lib64/codeblocks file per file, have make uninstall just delete that entire directory (same for /usr/local/include/codeblocks and /usr/local/share/codeblocks) ?
In the "/usr/local/lib64/" we also have 4 files related to libcodeblocks and libwxsmith (so the can still suffer from name change, but their names seem rather stable).

So could we adjust the makefile uninstall to just remove those directories (rm -rf - wise) at the end ?

Another thing I noticed, wxsmith appears next to it like :
/usr/local/include/wxsmith/
Why is this not a subdir of codeblocks directory?

[Jenna]

It might indeed be a problem with the zipping-processes.
It copies the appropriate manifest-file tomanifest.xml ancd then runs zip to pack it, if several of these steps run in parallel they might override (or try to override) the manifest.xml that is just used by another zip process.

This should be fixable by making the zip-processes depend on each other.

I will look into it as soon as possible.

[MortenMacFly]

I will look into it as soon as possible.

A very easy solution would be to move the manifest into sub-folders "HDL", "Cpp".... Then you don't need to rename and can run the ZIP commands in parallel w/o hassle.

[oBFusCATed]

you are correct I have run into issues, but that was very often pch problems (which seem to be the very nature of pch builds) 
and probably a few I don't remember, I can't even remember which wrong commands I used (could you please point them out to me again, would be good to have my mind refreshed    )

Doesn't matter what kind of issues you've been having. I've almost never had issues with the package generators, because they know how to build software reliably and then install/uninstall it correctly.
In gentoo the two steps are tied together, but in rpm based distros is a tree step activity or two (if you make a repo, but I don't want to bother with this).

But never such problem where actually the build process itself is screwing up things (like those manifest files / zip files getting broken (?) for SmartIndent with parallel build).

Probably this is an exception, but the break-ability of manual make based builds is still there, especially if you don't use non-default prefix.

But never such problem where actually the build process itself is screwing up things (like those manifest files / zip files getting broken (?) for So could we adjust the makefile uninstall to just remove those directories (rm -rf - wise) at the end ?

No, because it is not safe. Stick to packages for healthy linux systems. This is the general rule. Every system maintainer will tell you this. This is of course if you want to be able to update the system. If you prefer to reinstall the whole system often doesn't matter much.

[killerbot]

so basically what you are saying is, don't do sudo make install anymore.
But even when I specify a prefix, the problem will remain, since the uninstall in that area will also not clean up stuff correctly. It is however does manually delete things then.

Aside from that, I really do think, that a makefile should always clean up whatever it generated, even leaving empty directories is not nice.

Well I might go to a prefixe-ed cb install, but still based upon make install, because I am the only user on the system, it will be no problem that the other accounts will be unable to run it ;-)

I have no idea, how to end up with packages locally, could you give me some information on this ? (Opensuse)

[oBFusCATed]

so basically what you are saying is, don't do sudo make install anymore.

Yes, at least I don't do it

But even when I specify a prefix, the problem will remain, since the uninstall in that area will also not clean up stuff correctly. It is however does manually delete things then.

I don't use make uninstall, but a straight delete...

Aside from that, I really do think, that a makefile should always clean up whatever it generated, even leaving empty directories is not nice.

Probably there are errors or this is just how it works. I won't bother to fix it as I never use it. I would even vote to remove it if we make a poll.
at the other accounts will be unable to run it ;-)

I have no idea, how to end up with packages locally, could you give me some information on this ? (Opensuse)

Search the internet, I'm not sure what is used by opensuse for the building, but I'm sure there are some docs.
You can even start to provide opensuse repo for the nightlies.

[Jenna]

You can even start to provide opensuse repo for the nightlies.

Looks like it already exists: http://forums.codeblocks.org/index.php/topic,17038.msg116402.html#msg116402

[Jenna]

I will look into it as soon as possible.

A very easy solution would be to move the manifest into sub-folders "HDL", "Cpp".... Then you don't need to rename and can run the ZIP commands in parallel w/o hassle.

@killerbot:

can you test the following patch, please:

Code

Index: src/plugins/contrib/SmartIndent/Makefile.am
===================================================================
--- src/plugins/contrib/SmartIndent/Makefile.am
+++ src/plugins/contrib/SmartIndent/Makefile.am
@@ -99,6 +99,8 @@

 CLEANFILES = $(pkgdata_DATA)

+.NOTPARALLEL: $(pkgdata_DATA)
+
 SmartIndentHDL.zip: manifest_hdl.xml
        PWD=`pwd` cd $(srcdir) && \
        cp manifest_hdl.xml ./manifest.xml && \

I can build with it, but there haven't been any issues before (with -j 2).
I tested with -j 4, but I have just a core2duo, so I might not have enough cores to break the build.

[Alpha]

Code

+.NOTPARALLELL: $(pkgdata_DATA)

Slightly off topic, but "parallel" is spelled with only one "l" at the end (so I assume this means a double "l" ending is the secret code used throughout the build system ).

[Jenna]

Code

+.NOTPARALLELL: $(pkgdata_DATA)

Slightly off topic, but "parallel" is spelled with only one "l" at the end (so I assume this means a double "l" ending is the secret code used throughout the build system ).

Oops, thank you.
I corrected the patch above.

[killerbot]

yes, that works, I have committed this fix.