Bug 114041

Summary: kbabel does not recognize "#: REFERENCE" comments (single .po file)
Product: [Unmaintained] kbabel Reporter: Daniel Hahler <kde-bugzilla>
Component: generalAssignee: Stanislav Visnovsky <visnovsky>
Status: RESOLVED FIXED    
Severity: normal CC: nicolasg
Priority: NOR    
Version: 1.10.2   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Daniel Hahler 2005-10-07 21:45:40 UTC
Version:           1.10.2 (using KDE KDE 3.4.90)
Installed from:    Unlisted Binary Package

I have a .po file with comments like this:

---------------------------------------------
#: ../../../skins/natural_pink/_main.php:78
#: ../../../skins/nifty_corners/_main.php:100
msgid "Categories"
msgstr "Kategorien"
---------------------------------------------

Unfortunately KBabel displays them only in the "Comment" field (as is) and says "Corresponding source file not found".

The paths are of course correct and relative to the messages.po file.
Comment 1 Daniel Hahler 2005-10-07 21:50:03 UTC
I've meant:
..and says "Corresponding source file not found" in the "Source" tab.
Comment 2 Daniel Hahler 2005-10-12 22:14:45 UTC
It would be nice also, if backslashes (Windows) would get "translated" to standard unix forward slashes. I mean: KBabel should understand both formats.
Comment 3 Nicolas Goutte 2005-10-16 16:38:44 UTC
Can you create a separate bug report for the wish in comment #2 please?

As for the bug reported here, I can only suspect that somewhere in the code the .. directory is disallowed "for security reasons". (Not sure if this kind of security should apply to KBabel.)

Have a nice day!

Comment 4 Daniel Hahler 2005-10-16 19:24:26 UTC
I've just tried it with the full path to the .php file (also without ":linenr" at the end), and it also cannot find "the corresponding source file".

There seems to be something else wrong with it..

Because I do not know if the '/' == '\' feature in comment #2 is already there I will not open a report for it yet.
Comment 5 Nicolas Goutte 2005-11-07 17:23:51 UTC
To comment #4: if a full path does not work, it could mean too that there is a blocking "security feature" somewhere. (I had not the time to find it yet.)

Have a nice day!
Comment 6 Nicolas Goutte 2005-11-07 20:03:31 UTC
I have looked a little more in the code.

To comment #4: an absolute path does not work, as the path given in the comment of a PO file is put into a complex expression (which is set in Project/Configure), so put in that expression, the path is not absolute anymore.

To comment #2: as Qt's class QFileInfo is used backslashes as directory separators will probably only work under MS Windows. So you can add a wish/bug report.

Otherwise, I do not find anything that would block relative paths. (But the code is probably not as clean as it could be, so perhaps the problem is subtle.)

Perhaps you should (double-)check that the settings about the source referencing in Project/Configure are correct.

Have a nice day!
Comment 7 Daniel Hahler 2005-11-15 02:31:56 UTC
Thanks, Nicolas!

The problem was that I had not configured anything in Project/Configure/Source, because I just have a single .po file and have assumed that it will use paths relative to this .po file.

Regarding comment #2, I've simply tested it by editing the comments and it does not work. I've just filed http://bugs.kde.org/show_bug.cgi?id=116393.

I've also just found bug #86713 where getting the source files configured right is mentioned as a usability problem. Either file this bug as a DUPE for that one, or close it.

Thanks again.
Comment 8 Nicolas Goutte 2005-11-16 20:55:02 UTC
On Tuesday 15 November 2005 02:31, dAniel hAhler wrote:
[bugs.kde.org quoted mail]

But is the path valid compared to the location of the PO file?
(If yes, is it a certain open source project?)

>
> Regarding comment #2, I've simply tested it by editing the comments and it
> does not work. I've just filed http://bugs.kde.org/show_bug.cgi?id=116393.


Thank you!

>

(...)
>
> Thanks again.


Have a nice day!
Comment 9 Daniel Hahler 2005-11-16 21:17:27 UTC
The project is http://b2evolution.net, a php blog application.
The files are created like this (with paths relative to the .po file) since I've joined development - I've thought this was common.
Comment 10 Nicolas Goutte 2005-11-16 23:53:43 UTC
On Wednesday 16 November 2005 21:17, dAniel hAhler wrote:
(...)
> The project is http://b2evolution.net, a php blog application.
> The files are created like this (with paths relative to the .po file) since
> I've joined development - I've thought this was common.


I have checked. the Gettext's source is not made so either. The relative path 
in the PO files will not match the position of the PO file.

But doing so is perhaps intuitive and so KBabel should be able to support such 
a mode.

(As for KBabel's defaults, there are KDE-related, so if your POT and PO files 
are not made like KDE's you willprobably have to tweak a few things in 
KBabel's configuration.)

Have a nice day!
Comment 11 Daniel Hahler 2005-11-17 00:15:41 UTC
I was using poedit before under Win32, but on Linux it's plain ugly (GTK font issues).

Back in my windows days I was eager to try KBabel, because I've heard so many good things about it.

Having "relative path" as source path selector would be good IMHO.
Comment 12 Nicolas Goutte 2005-11-28 16:30:19 UTC
On Thursday 17 November 2005 00:15, dAniel hAhler wrote:
(...)
>
> Having "relative path" as source path selector would be good IMHO.


Why looking for something else in the Gettext info pages, I have found that 
Emacs PO mode works that way as first step.

However there is also a second step: to try to find the directory starting 
from the parent directory.

So if we have a reference to path/file.cpp we should search:
./path/file.cpp
../path/file.cpp

Have a nice day!
Comment 13 Nicolas Goutte 2005-12-25 11:36:09 UTC
SVN commit 491213 by goutte:

- add a new variable @POFILEDIR@ for the source reference
- improve the Doxygen comment of the SourceContext class to explain
  what each variable does

The new variable is to allow the user to easily specify paths relative
to the directory where the current PO file is. This is especially useful
for non-KDE projects.
(It is the main step to fix bug #114041.)
CCBUG:114041


 M  +22 -3     commonui/context.cpp  
 M  +33 -10    commonui/context.h  
 M  +1 -2      kbabel/sourceview.cpp  


--- branches/KDE/3.5/kdesdk/kbabel/commonui/context.cpp #491212:491213
@@ -50,6 +50,7 @@
 #include <kdialog.h>
 #include <klocale.h>
 #include <kmessagebox.h>
+#include <kurl.h>
 
 #include <klibloader.h>
 #include <ktrader.h>
@@ -73,11 +74,11 @@
     _layout->addWidget(_referenceCombo);
 }
 
-void SourceContext::setContext(const QString& packageDir, const QString& packageName, const QString& gettextComment)
+void SourceContext::setContext( const QString& packageDir, const QString& packageName, const QString& gettextComment, const KURL& urlPoFile )
 {
     if( !_part && !loadPart() ) return;
     _referenceCombo->clear();
-    _referenceList = resolvePath(packageDir,packageName,gettextComment);
+    _referenceList = resolvePath( packageDir, packageName, gettextComment, urlPoFile );
     
     for( QValueList<ContextInfo>::const_iterator it = _referenceList.constBegin(); it != _referenceList.constEnd(); ++it )
 	_referenceCombo->insertItem((*it).path);
@@ -115,9 +116,19 @@
     (dynamic_cast<KTextEditor::SelectionInterface *>(_part))->setSelection(ci.line-1,0,ci.line,0);
 }
 
-QValueList<ContextInfo> SourceContext::resolvePath(const QString& packageDir, const QString& packageName, const QString& gettextComment)
+QValueList<ContextInfo> SourceContext::resolvePath( const QString& packageDir, const QString& packageName, const QString& gettextComment, const KURL& urlPoFile )
 {
     //kdDebug() << "GETTEXTCOMMENT:" << gettextComment << endl;
+
+    // Find the directory name of the PO file, if the PO file is local
+    QString poDir;
+    if ( urlPoFile.isLocalFile() )
+    {
+        const QFileInfo fi( urlPoFile.path() );
+        poDir = fi.dirPath( true );
+        kdDebug() << "PO FILE DIR: " << poDir  << endl;
+    }
+    
     QStringList prefixes;
     QStringList paths = _project->settings()->paths();
     
@@ -125,6 +136,14 @@
 	; it!=paths.end() ; ++it)
     {
 	QString pref = (*it);
+
+        if ( !poDir.isEmpty() )
+        {
+            pref.replace( "@POFILEDIR@", poDir );
+        }
+        else if ( pref.find( "@POFILEDIR@ " ) != -1 )
+            continue; // No need to keep this path pattern, as we have no PO file dir
+        
         pref.replace( "@PACKAGEDIR@", packageDir);
         pref.replace( "@PACKAGE@", packageName);
 	pref.replace( "@CODEROOT@", _project->settings()->codeRoot());
--- branches/KDE/3.5/kdesdk/kbabel/commonui/context.h #491212:491213
@@ -46,6 +46,7 @@
 class QLineEdit;
 class KListEditor;
 class KConfig;
+class KURL;
 
 struct ContextInfo
 {
@@ -54,14 +55,25 @@
 };
 
 /**
-* Widget for displaying source code context of for the given GNU gettext comment.
-* The searched paths can be configured using variables @PACKAGE@, @PACKAGEDIR@,
-* @CODEROOT@ and @COMMENTPATH@. It requires a KPart implementing KTextEditor interface
-* with selections.
-*
-* @short Class for displaying source code context
-* @author Stanislav Visnovsky <visnovsky@kde.org>
-*/
+ * @short Class for displaying source code context
+ *
+ * Widget for displaying source code context of for the given GNU gettext comment.
+ * The searched paths can be configured using variables.
+ *
+ * The possible variables are:
+ * - \@POFILEDIR\@   absolute directory of the PO file (to create paths relatives to the PO file)
+ * - \@PACKAGE\@     name of the PO file
+ * - \@PACKAGEDIR\@  relative directory of the PO file (relative to \@CODEROOT\@)
+ * - \@CODEROOT\@    base directory (especially of the catalog manager)
+ * - \@COMMENTPATH\@ (relative) path given as source code reference in a comment of the PO file
+ *
+ * @note The difference between \@POFILEDIR\@ and a path constructed by
+ * \@CODEROOT\@\@PACKAGEDIR\@/ is that \@POFILEDIR\@ will also work if the file is external
+ * to the catalog manager's root
+ *
+ * @note It requires a KPart implementing KTextEditor interface with selections.
+ * @author Stanislav Visnovsky <visnovsky@kde.org>
+ */
 class SourceContext : public QWidget
 {
     Q_OBJECT
@@ -76,11 +88,22 @@
      * @param packageDir	path of the package, where to find the source file
      * @param packageName	name of the package, where to find the source file
      * @param gettextComment	comment string with context as generated by xgettext (can start with #:)
+     * @param urlPoFile         URL of the PO file
+     * @todo even if @p urlPoFile is an URL SourceContext::resolvePath is not remote-aware yet
      */
-    void setContext( const QString& packageDir, const QString& packageName, const QString& gettextComment );
+    void setContext( const QString& packageDir, const QString& packageName, const QString& gettextComment, const KURL& urlPoFile );
 
 private:
-    QValueList<ContextInfo> resolvePath( const QString& packageDir, const QString& packageName, const QString& gettextComment);
+    /**
+     * Get a list of paths from the source references in the comment @p gettextComment
+     * @param packageDir        path of the package, where to find the source file
+     * @param packageName       name of the package, where to find the source file
+     * @param gettextComment    comment string with context as generated by xgettext (can start with #:)
+     * @param urlPoFile         URL of the PO file
+     * @todo even if @p urlPoFile is an URL SourceContext::resolvePath is not remote-aware yet
+     * @private 
+     */
+    QValueList<ContextInfo> resolvePath( const QString& packageDir, const QString& packageName, const QString& gettextComment, const KURL& urlPoFile );
     bool loadPart();
     
     KTextEditor::Document* _part;
--- branches/KDE/3.5/kdesdk/kbabel/kbabel/sourceview.cpp #491212:491213
@@ -62,8 +62,7 @@
     if (isVisible ())
     {
         // Note: we use Catalog::comment instead of Catalog::context as SourceContext::setContext has to repeat the major part of the code of Catalog::context, so SourceContext::setContext can do the whole job alone.
-	_contextView->setContext(_catalog->packageDir()
-            ,_catalog->packageName(), _catalog->comment(_currentIndex));
+	_contextView->setContext( _catalog->packageDir(), _catalog->packageName(), _catalog->comment(_currentIndex), _catalog->currentURL() );
     }
 }
 
Comment 14 Nicolas Goutte 2005-12-25 12:33:00 UTC
SVN commit 491295 by goutte:

Search the refered files at two other places:
- relative to directory where the PO file is
- relative to the parent of the directory where the PO file is
  (This is the habit of GNU projects.)
CCBUG:114041
This should fix bug #114041 but I have not tested yet.


 M  +1 -1      kbprojectsettings.kcfg  


--- branches/KDE/3.5/kdesdk/kbabel/common/kbprojectsettings.kcfg #491294:491295
@@ -348,7 +348,7 @@
 		<default></default>
 	</entry>
 	<entry name="Paths" type="StringList">
-		<default>@PACKAGEDIR@/@PACKAGE@/@COMMENTPATH@,@CODEROOT@/@PACKAGEDIR@/@PACKAGE@/@COMMENTPATH@,@CODEROOT@/@PACKAGE@/@COMMENTPATH@</default>
+		<default>@PACKAGEDIR@/@PACKAGE@/@COMMENTPATH@,@CODEROOT@/@PACKAGEDIR@/@PACKAGE@/@COMMENTPATH@,@CODEROOT@/@PACKAGE@/@COMMENTPATH@,@POFILEDIR@/@COMMENTPATH@,@POFILEDIR@/../@COMMENTPATH@</default>
 	</entry>
   </group>
 </kcfg>
Comment 15 Nicolas Goutte 2005-12-26 10:49:05 UTC
SVN commit 491441 by goutte:

As we can process only local files for @POFILEDIR@ process the PO file URL
by KIO::NetAccess::mostLocalURL (of course only if compile on KDE >= 3.5)
BUG:114041
(Bug #114041 is done, even without this commit. Tested with de.po of svn 1.2.3)


 M  +14 -1     context.cpp  
 M  +3 -1      context.h  


--- branches/KDE/3.5/kdesdk/kbabel/commonui/context.cpp #491440:491441
@@ -51,6 +51,8 @@
 #include <klocale.h>
 #include <kmessagebox.h>
 #include <kurl.h>
+#include <kdeversion.h>
+#include <kio/netaccess.h>
 
 #include <klibloader.h>
 #include <ktrader.h>
@@ -60,6 +62,7 @@
 #include <ktexteditor/viewcursorinterface.h>
 
 SourceContext::SourceContext(QWidget *parent, KBabel::Project::Ptr project): QWidget(parent)
+    , m_parent( parent )
     , _part(0)
     , _view(0)
     , _referenceCombo(0)
@@ -121,13 +124,23 @@
     //kdDebug() << "GETTEXTCOMMENT:" << gettextComment << endl;
 
     // Find the directory name of the PO file, if the PO file is local
+    // ### TODO: find a way to allow remote files too
     QString poDir;
+#if KDE_IS_VERSION( 3, 5, 0 )
+    const KURL localUrl( KIO::NetAccess::mostLocalURL( urlPoFile, m_parent ) );
+    if ( localUrl.isLocalFile() )
+    {
+        const QFileInfo fi( localUrl.path() );
+        poDir = fi.dirPath( true );
+    }
+#else
     if ( urlPoFile.isLocalFile() )
     {
         const QFileInfo fi( urlPoFile.path() );
         poDir = fi.dirPath( true );
-        kdDebug() << "PO FILE DIR: " << poDir  << endl;
     }
+#endif
+    //kdDebug() << "PO FILE DIR: " << poDir  << endl;
     
     QStringList prefixes;
     QStringList paths = _project->settings()->paths();
--- branches/KDE/3.5/kdesdk/kbabel/commonui/context.h #491440:491441
@@ -105,7 +105,9 @@
      */
     QValueList<ContextInfo> resolvePath( const QString& packageDir, const QString& packageName, const QString& gettextComment, const KURL& urlPoFile );
     bool loadPart();
-    
+
+    /// Parent widget (for KIO::NetAccess member functions)
+    QWidget* m_parent;
     KTextEditor::Document* _part;
     KTextEditor::View* _view;
     QComboBox *_referenceCombo;