Discussion:
Moving LyX XMPP Chat to features/chat2
Tommaso Cucinotta
2013-11-12 22:21:48 UTC
Permalink
Kornel,

I'm just thinking of dropping entirely features/chat, and
continuing on features/chat2.

Not sure about what happened with features/chat, but from
the web view I can only say it seems a critical and unrecoverable
spaghetti situation :D -- I have no clue of how I could do it!

On the other hand, features/chat2 seems OK and I recently added
a couple of clean-up patches that also fix whitespaces compliance
with LyX coding style (you want to checkout this branch, before
any further edits, to prevent conflicts).

T.
Kornel Benko
2013-11-13 09:02:30 UTC
Permalink
Post by Tommaso Cucinotta
Kornel,
I'm just thinking of dropping entirely features/chat, and
continuing on features/chat2.
No problem. I have only sk.po changed here.
Post by Tommaso Cucinotta
Not sure about what happened with features/chat, but from
the web view I can only say it seems a critical and unrecoverable
spaghetti situation :D -- I have no clue of how I could do it!
On the other hand, features/chat2 seems OK and I recently added
a couple of clean-up patches that also fix whitespaces compliance
with LyX coding style (you want to checkout this branch, before
any further edits, to prevent conflicts).
I will.
Post by Tommaso Cucinotta
T.
Kornel
Kornel Benko
2013-11-13 09:25:35 UTC
Permalink
Post by Kornel Benko
Post by Tommaso Cucinotta
Kornel,
I'm just thinking of dropping entirely features/chat, and
continuing on features/chat2.
No problem. I have only sk.po changed here.
Post by Tommaso Cucinotta
Not sure about what happened with features/chat, but from
the web view I can only say it seems a critical and unrecoverable
spaghetti situation :D -- I have no clue of how I could do it!
On the other hand, features/chat2 seems OK and I recently added
a couple of clean-up patches that also fix whitespaces compliance
with LyX coding style (you want to checkout this branch, before
any further edits, to prevent conflicts).
I will.
Got problems with chat2 (by 'git checkout features/chat2').

...
From git.lyx.org:developers/tommaso/lyx
+ 17db3df...8da0eea features/chat2 -> origin/features/chat2 (forced update)
8f77d19..4963f71 features/chat -> origin/features/chat
6f488a9..be46813 master -> origin/master
Auto-merging src/frontends/qt4/ui/BuddiesUi.ui
CONFLICT (add/add): Merge conflict in src/frontends/qt4/ui/BuddiesUi.ui
Auto-merging src/frontends/qt4/GuiChatMessenger.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiChatMessenger.cpp
Auto-merging src/frontends/qt4/GuiChat.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiChat.cpp
Auto-merging src/frontends/qt4/GuiBuddies.h
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiBuddies.h
Auto-merging src/frontends/qt4/GuiBuddies.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiBuddies.cpp
Automatic merge failed; fix conflicts and then commit the result.

Deleting branch features/chat does not help

I then restarted with completely new checkout, takes ages, but was successful.
Post by Kornel Benko
Post by Tommaso Cucinotta
T.
Kornel
Vincent van Ravesteijn
2013-11-13 09:31:53 UTC
Permalink
Am Mittwoch, 13. November 2013 um 10:02:30, schrieb Kornel Benko <
Am Dienstag, 12. November 2013 um 22:21:48, schrieb Tommaso Cucinotta <
Post by Tommaso Cucinotta
Kornel,
I'm just thinking of dropping entirely features/chat, and
continuing on features/chat2.
No problem. I have only sk.po changed here.
Post by Tommaso Cucinotta
Not sure about what happened with features/chat, but from
the web view I can only say it seems a critical and unrecoverable
spaghetti situation :D -- I have no clue of how I could do it!
On the other hand, features/chat2 seems OK and I recently added
a couple of clean-up patches that also fix whitespaces compliance
with LyX coding style (you want to checkout this branch, before
any further edits, to prevent conflicts).
I will.
Got problems with chat2 (by 'git checkout features/chat2').
...
From git.lyx.org:developers/tommaso/lyx
+ 17db3df...8da0eea features/chat2 -> origin/features/chat2 (forced update)
8f77d19..4963f71 features/chat -> origin/features/chat
6f488a9..be46813 master -> origin/master
Auto-merging src/frontends/qt4/ui/BuddiesUi.ui
CONFLICT (add/add): Merge conflict in src/frontends/qt4/ui/BuddiesUi.ui
Auto-merging src/frontends/qt4/GuiChatMessenger.cpp
CONFLICT (add/add): Merge conflict in
src/frontends/qt4/GuiChatMessenger.cpp
Auto-merging src/frontends/qt4/GuiChat.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiChat.cpp
Auto-merging src/frontends/qt4/GuiBuddies.h
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiBuddies.h
Auto-merging src/frontends/qt4/GuiBuddies.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiBuddies.cpp
Automatic merge failed; fix conflicts and then commit the result.
Deleting branch features/chat does not help
I then restarted with completely new checkout, takes ages, but was successful.
The errors will not be the result of a 'git checkout' command. It more
looks like a git pull. This is the reason, I don't advice to use git pull.

If you would have done a 'git fetch', you will retrieve the new commits.
Then, you want to reset the features/chat2 branch to the new one tommasso
pushed:

$ git checkout features/chat2
$ git reset --hard <name_of_remote>/features/chat2

This must work without problems.

Vincent
Tommaso Cucinotta
2013-11-14 10:43:57 UTC
Permalink
Post by Vincent van Ravesteijn
If you would have done a 'git fetch', you will retrieve the new
commits. Then, you want to reset the features/chat2 branch to the new
$ git checkout features/chat2
$ git reset --hard <name_of_remote>/features/chat2
On a machine of a user who never checked out features/chat2 before, the first command would fail, wouldn't it ?

Don't you need to create first the branch locally, with smth like

git checkout -b features/chat2

or

git branch features/chat2

then the reset command ? (that forces the local branch to copy the status of the remote one)

Btw, Vincent, if you like, please, have a look at features/chat2 and provide comments. That seems pretty usable now.
One issue I'm aware of is: in case the user creates multiple views (File->New Window), and starts the chat in multiple windows, I want a per-GuiView instance of the whole chat machinery. However, as of now, the chat is actually a kind of single-ton (there's only one instance of QxmppClient for the whole LyX process). Do I need to modify the GuiView interface to allow for retrieval of this View-specific chat instance ? I need that because I have two independent panels (the buddies list on the left and the chat bar on the bottom) that have to be tied together.... per-view.

Any advise ?

Thx.

T.
Vincent van Ravesteijn
2013-11-14 15:15:17 UTC
Permalink
Post by Tommaso Cucinotta
Post by Vincent van Ravesteijn
If you would have done a 'git fetch', you will retrieve the new
commits. Then, you want to reset the features/chat2 branch to the new
$ git checkout features/chat2
$ git reset --hard <name_of_remote>/features/chat2
On a machine of a user who never checked out features/chat2 before, the first command would fail, wouldn't it ?
Don't you need to create first the branch locally, with smth like
git checkout -b features/chat2
or
git branch features/chat2
then the reset command ? (that forces the local branch to copy the status of the remote one)
No. If you fetch from the remote, the local repo knows about the remote
branches:

tommaso/features/chat2

If you then checkout the branch

$ git checkout features/chat2
Branch features/chat2 set up to track remote branch features/chat2 from
tommaso.
Switched to a new branch 'features/chat2'

So, no need to create the branch yourself.
Post by Tommaso Cucinotta
Btw, Vincent, if you like, please, have a look at features/chat2 and provide comments. That seems pretty usable now.
One issue I'm aware of is: in case the user creates multiple views (File->New Window), and starts the chat in multiple windows, I want a per-GuiView instance of the whole chat machinery. However, as of now, the chat is actually a kind of single-ton (there's only one instance of QxmppClient for the whole LyX process). Do I need to modify the GuiView interface to allow for retrieval of this View-specific chat instance ? I need that because I have two independent panels (the buddies list on the left and the chat bar on the bottom) that have to be tied together.... per-view.
Any advise ?
I don't have now (I'm busy preparing for an important academic event
tomorrow ;)).

Vincent
Kornel Benko
2013-11-15 16:58:25 UTC
Permalink
Post by Vincent van Ravesteijn
The errors will not be the result of a 'git checkout' command. It more
looks like a git pull. This is the reason, I don't advice to use git pull.
If you would have done a 'git fetch', you will retrieve the new commits.
Then, you want to reset the features/chat2 branch to the new one tommasso
$ git checkout features/chat2
$ git reset --hard <name_of_remote>/features/chat2
This must work without problems.
Vincent
Sure Vincent. If one knows how.

For some days I was not looking at this branch.
I am really trying hard but:

#git reset --hard features/chat2
HEAD is now at 2993f12 whitespaces
#git fetch
#git status
# On branch features/chat2
# Your branch and 'origin/features/chat2' have diverged,
# and have 16 and 33 different commits each, respectively.
#
nothing to commit (working directory clean)
#git branch
* features/chat2
master
#git pull
Auto-merging src/frontends/qt4/GuiView.h
Auto-merging src/frontends/qt4/GuiView.cpp
Auto-merging src/frontends/qt4/GuiChatMessenger.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiChatMessenger.cpp
Auto-merging src/frontends/qt4/GuiChat.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiChat.cpp
Auto-merging src/frontends/qt4/GuiBuddies.h
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiBuddies.h
Auto-merging src/frontends/qt4/GuiBuddies.cpp
CONFLICT (add/add): Merge conflict in src/frontends/qt4/GuiBuddies.cpp
Auto-merging configure.ac
Automatic merge failed; fix conflicts and then commit the result.
Exit 1
#git status
# On branch features/chat2
# Your branch and 'origin/features/chat2' have diverged,
# and have 16 and 33 different commits each, respectively.
#
# Changes to be committed:
#
# modified: configure.ac
# modified: development/autotests/revertedTests
# modified: development/autotests/run-tests.sh
# modified: lib/layouts/paper.layout
# modified: src/Buffer.cpp
# modified: src/LaTeXFeatures.cpp
# modified: src/LyXAction.cpp
# modified: src/frontends/qt4/FileDialog.cpp
# modified: src/frontends/qt4/GuiHSpace.cpp
# modified: src/frontends/qt4/GuiVSpace.cpp
# modified: src/frontends/qt4/GuiView.cpp
# modified: src/frontends/qt4/GuiView.h
# modified: src/insets/InsetArgument.cpp
#
# Unmerged paths:
# (use "git add/rm <file>..." as appropriate to mark resolution)
#
# both added: src/frontends/qt4/GuiBuddies.cpp
# both added: src/frontends/qt4/GuiBuddies.h
# both added: src/frontends/qt4/GuiChat.cpp
# both added: src/frontends/qt4/GuiChatMessenger.cpp
#

What the mockery ... Nothing I try helps here. I for sure did not commit or change anything.

Kornel
Tommaso Cucinotta
2013-11-15 19:56:18 UTC
Permalink
Post by Kornel Benko
What the mockery ... Nothing I try helps here. I for sure did not commit or change anything.
do u mind sending the part of your .lyx/config defining your "origin" repo, as well as your
"features/chat2" branch ?

Thx,

T.
Kornel Benko
2013-11-15 20:24:50 UTC
Permalink
Post by Tommaso Cucinotta
Post by Kornel Benko
What the mockery ... Nothing I try helps here. I for sure did not commit or change anything.
do u mind sending the part of your .lyx/config defining your "origin" repo, as well as your
"features/chat2" branch ?
Thx,
T.
I made a new clone. Compared with the the previous config, no differences.
The new checkout behaves OK for now.
Attached the previous one.

Also the global one, ~/.gitconfig, does not seem to be bad.

The branch is big.

#du -s lyx.chat.old/.
302540 lyx.chat.old/.

#du -s lyx/. (this is the new lyx-chat)
302944 lyx/.

I probably misunderstood, what to send.

Kornel
Tommaso Cucinotta
2013-11-16 00:49:16 UTC
Permalink
Post by Kornel Benko
I probably misunderstood, what to send.
it was ok. I just wanted to check whether your origin was actually the main lyx repo or mine, so you actually must have cloned the whole repo developers/tommaso as an independent folder (calling it as origin), that's fine.

I guess it's all right from your side. The one to blame is just me, as I've been keeping features/chat2 clean and rebased on top of master (a.k.a., lyx trunk).
As I have "rebase=true" for this branch in my .git/config, it rebases every time I pull, but then I also needed push -f quite a number of times, even though I don't do that unless I double-checked there are no other commits from you on the repo (in which case I pulled these commits in my local branch before pushing).

As Vincent pointed out, rebase rewrites the history, so it creates problem to whoever wants to follow the branch with a simple pull/merge. However, still it should be able to follow the branch with some additional reset.

The ideal behaviour should be refraining entirely from pulling additional patches from trunk (origin/master), but keeping developing features/chat2 on top of the point where it branched (or where it's been rebased so far), without pulling any further patches, but finishing it, then ask a merge of it into trunk. Kind of guidelines also quite clear from, e.g.:

http://lwn.net/Articles/328436/

however, as it's the first time I'm publishing the patch updates through a git branch (and don't know about others who also committed some patches there), rather than sending the patch by e-mail or attaching it to a ticket, this is all being a new and learning experience.

T.
Kornel Benko
2013-11-16 13:26:24 UTC
Permalink
Post by Tommaso Cucinotta
Post by Kornel Benko
I probably misunderstood, what to send.
it was ok. I just wanted to check whether your origin was actually the main lyx repo or mine, so you actually must have cloned the whole repo developers/tommaso as an independent folder (calling it as origin), that's fine.
I guess it's all right from your side. The one to blame is just me, as I've been keeping features/chat2 clean and rebased on top of master (a.k.a., lyx trunk).
As I have "rebase=true" for this branch in my .git/config, it rebases every time I pull, but then I also needed push -f quite a number of times, even though I don't do that unless I double-checked there are no other commits from you on the repo (in which case I pulled these commits in my local branch before pushing).
As Vincent pointed out, rebase rewrites the history, so it creates problem to whoever wants to follow the branch with a simple pull/merge. However, still it should be able to follow the branch with some additional reset.
http://lwn.net/Articles/328436/
Thanks for the pointer.
Post by Tommaso Cucinotta
however, as it's the first time I'm publishing the patch updates through a git branch (and don't know about others who also committed some patches there), rather than sending the patch by e-mail or attaching it to a ticket, this is all being a new and learning experience.
T.
Kornel
Vincent van Ravesteijn
2013-11-18 08:44:20 UTC
Permalink
Post by Tommaso Cucinotta
Post by Kornel Benko
I probably misunderstood, what to send.
it was ok. I just wanted to check whether your origin was actually the
main lyx repo or mine, so you actually must have cloned the whole repo
developers/tommaso as an independent folder (calling it as origin), that's
fine.
I guess it's all right from your side. The one to blame is just me, as
I've been keeping features/chat2 clean and rebased on top of master
(a.k.a., lyx trunk).
As I have "rebase=true" for this branch in my .git/config, it rebases
every time I pull, but then I also needed push -f quite a number of times,
even though I don't do that unless I double-checked there are no other
commits from you on the repo (in which case I pulled these commits in my
local branch before pushing).
As Vincent pointed out, rebase rewrites the history, so it creates problem
to whoever wants to follow the branch with a simple pull/merge. However,
still it should be able to follow the branch with some additional reset.
Yes, if Kornel is not very actively involved in refactoring stuff etc., it
should be well possible for him to reset once in a while, or to rebase his
local work onto your branch.
Post by Tommaso Cucinotta
The ideal behaviour should be refraining entirely from pulling additional
patches from trunk (origin/master), but keeping developing features/chat2
on top of the point where it branched (or where it's been rebased so far),
without pulling any further patches, but finishing it, then ask a merge of
http://lwn.net/Articles/328436/
however, as it's the first time I'm publishing the patch updates through a
git branch (and don't know about others who also committed some patches
there), rather than sending the patch by e-mail or attaching it to a
ticket, this is all being a new and learning experience.
Well, I hope my explanations will help in the learning.

Vincent
Tommaso Cucinotta
2013-11-20 00:51:04 UTC
Permalink
Post by Vincent van Ravesteijn
Yes, if Kornel is not very actively involved in refactoring stuff
etc., it should be well possible for him to reset once in a while, or
to rebase his local work onto your branch.
So I kept rebasing it a few further times...
Post by Vincent van Ravesteijn
Well, I hope my explanations will help in the learning.
Oh, we'd all be lost and have tons of inconsistent repos w/out u, Vincent :-)!

Btw, in features/chat2 I also fixed the multi-view problem. So, if you pull (with --reset I guess) now you can see that you can do

File->New Window

then launch via the menu the LyX chat

Tools->LyX Chat

in both windows (which are independent GuiView of the same LyX application), they'll keep by default the same login as the last you used, however you can change one of them, re-connect, then you can even talk from one LyX window to the other (going through a XMPP server, I know, a very meaningful scenario :-) ).

Should anyone feel like trying it out, you can start by adding myself (***@jabber.org) to your buddy list (you can do this via the "Add" button from within LyX itself now -- but you cannot register a new jabber.org account -- or others -- via LyX, I advise to use Pidgin for that).

Thx!

T.
Kornel Benko
2013-11-20 14:01:56 UTC
Permalink
Post by Tommaso Cucinotta
Post by Vincent van Ravesteijn
Yes, if Kornel is not very actively involved in refactoring stuff
etc., it should be well possible for him to reset once in a while, or
to rebase his local work onto your branch.
So I kept rebasing it a few further times...
Post by Vincent van Ravesteijn
Well, I hope my explanations will help in the learning.
Oh, we'd all be lost and have tons of inconsistent repos w/out u, Vincent :-)!
Btw, in features/chat2 I also fixed the multi-view problem. So, if you pull (with --reset I guess) now you can see that you can do
File->New Window
then launch via the menu the LyX chat
Tools->LyX Chat
in both windows (which are independent GuiView of the same LyX application), they'll keep by default the same login as the last you used, however you can change one of them, re-connect, then you can even talk from one LyX window to the other (going through a XMPP server, I know, a very meaningful scenario :-) ).
Thx!
T.
I like it. Feels good, especially on the fresh new window.

To get it into master should be the first think after releasing 2.1 IMHO.

Kornel
Tommaso Cucinotta
2013-12-22 14:52:09 UTC
Permalink
Post by Kornel Benko
I like it. Feels good, especially on the fresh new window.
To get it into master should be the first think after releasing 2.1 IMHO.
So, what's the current trunk situation ? I've just had some extra bits of time
to update/rebase the features/chat2 branch over the current trunk.

Xmas + new year days might be an excellent moment to apply some further desirable
changes, if needed...

In the mean-time, best wishes to everybody :-)!

T.
Tommaso Cucinotta
2014-04-28 00:19:57 UTC
Permalink
I like it. Feels good, especially on the fresh new window. To get it into master should be the first think after releasing 2.1 IMHO. Kornel
That seems the right time then!

Currently, this is a patchset of 21 commits

http://git.lyx.org/?p=developers/tommaso/lyx.git;a=shortlog;h=refs/heads/features/chat2

Guess Vincent might comment on whether these should be squashed into a single commit or similar?

T.
Vincent van Ravesteijn
2014-04-28 06:05:14 UTC
Permalink
Post by Tommaso Cucinotta
I like it. Feels good, especially on the fresh new window. To get it into
master should be the first think after releasing 2.1 IMHO. Kornel
That seems the right time then!
Currently, this is a patchset of 21 commits
http://git.lyx.org/?p=developers/tommaso/lyx.git;a=shortlog;h=refs/heads/features/chat2
Guess Vincent might comment on whether these should be squashed into a
single commit or similar?
T.
I wouldn't rush getting this into master. It would be nice to sort out
a few issues that I see for now:

- in config/qt4.m4 it seems to unconditionally pull in QtNetwork and
QtXml. Is this really so ?

- do we want to have a feature turned on by default that depends on
yet another dependency ?

- does it work on MacOSX ? Is libQXmpp installed by default on MacOSX
? Is it a problem to compile our own lib and supply it within the
package ? Did anyone test it already on MacOSX ?

- the same questions for Windows ?

- do we want to include this feature and all dependencies by default ?
Or supply separate binaries or whatever ?

- does anyone have general objections or concerns ?


Some more detailed remarks will follow.


Vincent
Vincent van Ravesteijn
2014-04-28 07:10:21 UTC
Permalink
Date: Wed, 16 Oct 2013 21:55:40 +0000 (+0100)
Subject: LyX XMPP Chat
http://git.lyx.org/?p=developers%2Ftommaso%2Flyx.git;a=commitdiff_plain;h=9646b7553bb4a3916e8b99f9e2dc200e7d534dfb
LyX XMPP Chat
---
@@ -186,7 +190,8 @@ SOURCEFILESCORE = \
VCBackend.cpp \
version.cpp \
VSpace.cpp \
- WordList.cpp
+ WordList.cpp \
+ moc_BufferView.cpp
HEADERFILESCORE = \
Author.h \
@@ -316,7 +321,7 @@ endif
######################### Qt stuff ##############################
-MOCHEADER = Compare.h
+MOCHEADER = Compare.h BufferView.h
if INSTALL_WINDOWS
Why is BufferView suddenly moc'ed while it hasn't changed ?
diff --git a/src/frontends/qt4/GuiBuddies.cpp
b/src/frontends/qt4/GuiBuddies.cpp
new file mode 100644
index 0000000..138d162
--- /dev/null
+++ b/src/frontends/qt4/GuiBuddies.cpp
@@ -0,0 +1,284 @@
+/**
+ * \file GuiBuddies.cpp
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Tommaso Cucinotta
+ *
+ * Full author contact details are available in file CREDITS.
+ */
+
+#include <config.h>
+
+#include "GuiBuddies.h"
+#include "GuiChat.h"
+#include "LyXRC.h"
+
+#include "GuiApplication.h"
+#include "GuiView.h"
+#include "GuiWorkArea.h"
+#include "qt_helpers.h"
+#include "Language.h"
+#include <QWidget>
+#include <QInputDialog>
+#include <QLineEdit>
+
+#include "FuncRequest.h"
+#include "lyxfind.h"
+
+#include "frontends/alert.h"
+#include "DispatchResult.h"
+
+#include "support/debug.h"
+#include "support/filetools.h"
+#include "support/FileName.h"
+#include "support/gettext.h"
+#include "support/lassert.h"
+#include "support/lstrings.h"
+#include "support/Package.h"
+
+#include <qxmpp/QXmppMessage.h>
+#include <qxmpp/QXmppRosterManager.h>
+
The sorting of the include could be improved a bit.

+static string availStatusToText(QXmppPresence::AvailableStatusType s) {
+ static const char *texts[] = { "Online", "Away", "Extended Away", "Do
Not Disturb", "Buddies", "Invisible" };
+ LASSERT(s <= sizeof(texts) / sizeof(texts[0]), /**/);
+ return texts[s];
+}
+
+
+static string presenceTypeToText(QXmppPresence::Type t) {
+ static const char *texts[] = { "Error", "Available", "Unavailable",
"Subscribe", "Subscribed", "Unsubscribe", "Unsubscribed", "Probe" };
+ LASSERT(t <= sizeof(texts) / sizeof(texts[0]), /**/);
+ return texts[t];
+}
One of the reasons to nicely squash together commits within a series is
that I wouldn't start commenting on this part of the patch as it is removed
later on. Now, I don't know that and I might start reviewing irrelevant
parts.
+
+
+static QListWidgetItem *findInListWidget(QListWidget *p_list, const
QString &s) {
We use to write "QString const & s".
+ for (int i = 0; i < p_list->count(); ++i)
+ if (p_list->item(i)->text() == s)
+ return p_list->item(i);
+ return 0;
+}
+
Would it be an idea to use QListWidget::findItems() ?

+void GuiBuddiesWidget::onStateSelected(const QString & qs)
+{
+ QXmppPresence p;
+ string s = fromqstr(qs);
+ if (s == "Available")
+ p.setAvailableStatusType(QXmppPresence::Online);
+ else if (s == "Away")
+ p.setAvailableStatusType(QXmppPresence::Away);
+ else if (s == "Do Not Disturb")
+ p.setAvailableStatusType(QXmppPresence::DND);
+ lyxerr << "Setting ";
+ dumpPres(p);
+ theChatMessenger()->setClientPresence(p);
+}
Why converting from QString to string before ?


diff --git a/src/frontends/qt4/GuiBuddies.h b/src/frontends/qt4/GuiBuddies.h
new file mode 100644
index 0000000..d85f131
--- /dev/null
+++ b/src/frontends/qt4/GuiBuddies.h
@@ -0,0 +1,105 @@
[..]
+#ifndef QGUIBUDDIES_H
+#define QGUIBUDDIES_H
QGUIBUDDIES_H ?


+
+#undef QT_NO_KEYWORDS
Why ?
+
+void GuiChatWidget::onMessageReceived(string const & from, string const &
s)
+{
+ printf("Received %lu bytes: %s\n", s.size(), s.c_str());
printf ?

diff --git a/src/frontends/qt4/GuiChat.h b/src/frontends/qt4/GuiChat.h
new file mode 100644
index 0000000..724815a
--- /dev/null
+++ b/src/frontends/qt4/GuiChat.h
@@ -0,0 +1,109 @@
+// -*- C++ -*-
+/**
+ * \file Chat.h
Chat.h ?
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Tommaso Cucinotta
+ *
+ * Full author contact details are available in file CREDITS.
+ */
+
+#ifndef QGUICHAT_H
+#define QGUICHAT_H
QGUICHAT_H ?
+
+#undef QT_NO_KEYWORDS
Why ?

diff --git a/src/frontends/qt4/GuiChatMessenger.h
b/src/frontends/qt4/GuiChatMessenger.h
new file mode 100644
index 0000000..f8e2b3c
--- /dev/null
+++ b/src/frontends/qt4/GuiChatMessenger.h
@@ -0,0 +1,53 @@
+// -*- C++ -*-
+/**
+ * \file Chat.h
Chat.h ?
+ * This file is part of LyX, the document processor.
+ * Licence details can be found in the file COPYING.
+ *
+ * \author Tommaso Cucinotta
+ *
+ * Full author contact details are available in file CREDITS.
+ */
+
+#ifndef QGUICHATMESSENGER_H
+#define QGUICHATMESSENGER_H
QGUICHATMESSENGER_H?
+
+#undef QT_NO_KEYWORDS
+
Why ?
diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp
index a7cbbe4..f397da7 100644
--- a/src/frontends/qt4/GuiView.cpp
+++ b/src/frontends/qt4/GuiView.cpp
@@ -1734,7 +1734,9 @@ bool GuiView::getStatus(FuncRequest const & cmd,
FuncStatus & flag)
|| name == "prefs"
|| name == "texinfo"
|| name == "progress"
- || name == "compare";
+ || name == "compare"
+ || name == "chat"
+ || name == "chat-bar";
else if (name == "print")
enable = doc_buffer->params().isExportable("dvi")
&& lyxrc.print_command != "none";
@@ -3957,7 +3959,7 @@ namespace {
char const * const dialognames[] = {
"aboutlyx", "bibitem", "bibtex", "box", "branch", "changes", "character",
-"citation", "compare", "comparehistory", "document", "errorlist", "ert",
+"chat", "chat-bar", "citation", "compare", "comparehistory", "document",
"errorlist", "ert",
"external", "file", "findreplace", "findreplaceadv", "float", "graphics",
"href", "include", "index", "index_print", "info", "listings", "label",
"line",
"log", "mathdelimiter", "mathmatrix", "mathspace", "nomenclature",
@@ -4144,6 +4146,8 @@ Dialog * createGuiAbout(GuiView & lv);
Dialog * createGuiBibtex(GuiView & lv);
Dialog * createGuiChanges(GuiView & lv);
Dialog * createGuiCharacter(GuiView & lv);
+Dialog * createGuiChat(GuiView & lv);
+Dialog * createGuiBuddies(GuiView & lv);
Dialog * createGuiCitation(GuiView & lv);
Dialog * createGuiCompare(GuiView & lv);
Dialog * createGuiCompareHistory(GuiView & lv);
@@ -4192,10 +4196,14 @@ Dialog * GuiView::build(string const & name)
return createGuiAbout(*this);
if (name == "bibtex")
return createGuiBibtex(*this);
+ if (name == "chat")
+ return createGuiBuddies(*this);
if (name == "changes")
return createGuiChanges(*this);
if (name == "character")
return createGuiCharacter(*this);
+ if (name == "chat-bar")
+ return createGuiChat(*this);
if (name == "citation")
return createGuiCitation(*this);
if (name == "compare")
"chat" refers to GuiBuddies, and "chat-bar" refers to GuiChat ? This looks
a bit confusing.
diff --git a/src/frontends/qt4/GuiView.h b/src/frontends/qt4/GuiView.h
index 17e60df..4e563d2 100644
--- a/src/frontends/qt4/GuiView.h
+++ b/src/frontends/qt4/GuiView.h
void hideDialog(std::string const & name, Inset * inset);
///
void disconnectDialog(std::string const & name);
+ ///
+ Dialog * findOrBuild(std::string const & name, bool hide_it);
/// Saves the layout and geometry of the window
/// Is the dialog currently visible?
bool isDialogVisible(std::string const & name) const;
///
- Dialog * findOrBuild(std::string const & name, bool hide_it);
- ///
Dialog * build(std::string const & name);
///
bool reloadBuffer(Buffer & buffer);
Why ? Please respect if a function is private to keep it private. GuiView
manages all dialogs, and widgets.

Can't you use doShowDialog ? This avoids handling the focus yourself etc.


Vincent
Tommaso Cucinotta
2014-05-04 21:13:46 UTC
Permalink
+ ///
+ Dialog * findOrBuild(std::string const & name, bool hide_it);
- Dialog * findOrBuild(std::string const & name, bool hide_it);
- ///
Dialog * build(std::string const & name);
Why ? Please respect if a function is private to keep it private. GuiView manages all dialogs, and widgets.
Can't you use doShowDialog ? This avoids handling the focus yourself etc.
Don't know -- doShowDialog() won't return me the pointer I need from GuiBuddies.cpp

GuiChat *p_dlg = dynamic_cast<GuiChat *>(view_.findOrBuild("chat-bar", false));

Perhaps going through view_.chatMessenger(), don't know, probably there are other
ways to organize the relationship among these 3 classes.

T.

Vincent van Ravesteijn
2014-04-28 07:48:43 UTC
Permalink
I like it. Feels good, especially on the fresh new window. To get it
into master should be the first think after releasing 2.1 IMHO. Kornel
That seems the right time then!
Currently, this is a patchset of 21 commits
http://git.lyx.org/?p=developers/tommaso/lyx.git;a=shortlog;h=refs/heads/features/chat2
Guess Vincent might comment on whether these should be squashed into a
single commit or similar?
Below are my opinions. Please read from bottom to top.

6 hours ago Tommaso Cucinotta Completed exclusion of chat from
GuiView when USE_QXMPP... features/chat2 commit | commitdiff | tree |
snapshot
Merge with first commit.
6 hours ago Kornel Benko * sk.po commit | commitdiff | tree |
snapshot
This is not part of this feature.
7 hours ago Tommaso Cucinotta Multi-view XMPP chat. commit |
commitdiff | tree | snapshot
Lose the "." in the commit subject. Further, I don't think that a Widget
should read/write files. The core functionality of the messenger should be
in "core" I guess. I'm not sure why GuiChatMessenger is called "Gui" and is
in frontends/qt4 as it doesn't have a related dialog or the like.

Why do you think it should be in frontends/qt4 ?
7 hours ago Tommaso Cucinotta Missing file-level comment for
GuiChatMessenger.cpp. commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Tommaso Cucinotta Polishing code, removing unused
functions, moving resid... commit |
Merge with first commit.
commitdiff | tree | snapshot
7 hours ago Tommaso Cucinotta whitespaces commit | commitdiff |
tree | snapshot
Merge with first commit.
7 hours ago Kornel Benko Make some GUI strings translatable
commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Tommaso Cucinotta LyX Chat - added a couple of missing
ifdefs in case... commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Tommaso Cucinotta Switch to new latexclipboard
importing string format... commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Tommaso Cucinotta Fix focus issue and add colors-based
distinction of... commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Tommaso Cucinotta Make chat menu item disappear in case
of no QXMPP support. commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Tommaso Cucinotta Fixed a few issues with id/resource
handling. commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Tommaso Cucinotta Made string/question translatable.
commit | commitdiff | tree | snapshot
Merge with first commit.
7 hours ago Kornel Benko Add option to cmake build for use (or not
use) of QXMPP commit | commitdiff | tree | snapshot
Merge with "Patch for compiling the QXMPP based chat with cmake..."
7 hours ago Tommaso Cucinotta LyX Chat - Let's see if this RETURN
problem is fixed. commit | commitdiff | tree | snapshot
I don't want to see this type of commits. Either merge with the first
commit, or explain exactly what the problem is that you try to fix here.
Are you now sure whether it fixes the problem ? If so, remove the "Let's
see", otherwise try to figure it out ;).
7 hours ago Tommaso Cucinotta LyX Chat - add/remove buddy
capability, plus various... commit | commitdiff | tree | snapshot
Various bugfixes should be merged with first commit. I would prefer to
either add the prefix "lyx-chat:" to all commits, or to none.
7 hours ago Tommaso Cucinotta Fix compilation problem when
compiling without qxmpp. commit | commitdiff | tree | snapshot
Merge with the first commit.
7 hours ago Tommaso Cucinotta Now presence icons are correctly
shown in my status... commit | commitdiff | tree | snapshot
Merge with the first commit.
7 hours ago Tommaso Cucinotta store and retrieval of user's
password from .passwd... commit | commitdiff | tree | snapshot
Commit subject line should consist of a single line without dots.
7 hours ago Tommaso Cucinotta Patch for compiling the QXMPP based
chat with cmake... commit | commitdiff | tree | snapshot
I propose the commit subject as: "lyx-chat: Compile QXMPP based chat with
CMake". The "Patch for" and "by Kornel Benko" parts are not very
informative for the subject.

The change to GuiBuddies.h should be merged with the previous commit.
7 hours ago Tommaso Cucinotta LyX XMPP Chat commit | commitdiff
| tree | snapshot
Vincent
Tommaso Cucinotta
2014-04-28 21:01:58 UTC
Permalink
Post by Vincent van Ravesteijn
Below are my opinions. Please read from bottom to top.
Hi,

thanks for these.

I see I definitely need a lyxchat3 rework before the merge.
I'll try to do that, but it will take one or a couple of weeks.

A few remarks:

-) Vincent suggests to create the new branch in features.git, so I'll ensure
the reworked patchset goes into a branch over there

-) I was expecting many late fixes to have to be merged to the first patch
from the beginning, I'll try to do that

-) possible concerns about distributing LyX: one such concerns may be due
to the fact that, in general, we add networking capabilities to LyX (this
may apply to the chat as well as to the interactive lyx feature); often
it's not enough to tell users that, until you don't use the chat, it won't
be able to hurt you; users might assume that now LyX is less secure
because it has a networking component, and if there's any bug, one
attacker might exploit a LyX weakness etc...
[ I'm especially thinking of a few people I know who use LyX to write
patent applications ]

So, one way to rule such concerns out, would be the one to have a
different executable: traditional lyx, with any networking feature
disabled at compile-time, versus collaborative lyx (e.g., with such
a name as "lyx-net" or similar), with said features enabled.

-) very good point the one to move GuiMessanger out of qt4, and s/Gui//

-) why the moc-ification of BufferView.h, and the QT_NO_KEYWORDS ?
I wish I remembered. Just, I was unable to compile otherwise, the
problem was due to pulling-in the QXMPP headers, I just found this
thread on lyx-devel explaining what happened at that time:

https://www.mail-archive.com/lyx-***@lists.lyx.org/msg181164.html

any hint as to how to deal properly/better with this problem would be
appreciated.

-) is QXMPP available on Windows and Mac OS-X ? No clue! Windows and
Mac people, please speak, and/or try to compile the tommaso/features/chat2
branch on Windows and Mac, and let's try to chat and see whether it works.
As for Windows, there's at least this thread
https://code.google.com/p/qxmpp/issues/detail?id=93
where there's at least one user claiming it compiles and works fine on WinXP 32bit,
and the thread starter complaining about problems with Win7 x64.

Thanks, bye.

Tommaso
Tommaso Cucinotta
2014-05-04 12:59:46 UTC
Permalink
Hi,

I just created a tommaso/features/chat3 new branch with a rework of the LyX chat feature, taking most of Vincent's comments. Most of the patches are now squashed into the initial commit.

For now the GuiChatMessenger stays in qt4/, as it relies on Qt libraries, and it also depends on GuiChat and calls one of its methods. It seemed not immediate to just move it to the core.

Vincent, any further issue that you can spot?

Thanks,

T.
Vincent van Ravesteijn
2014-04-28 09:36:21 UTC
Permalink
I like it. Feels good, especially on the fresh new window. To get it
into master should be the first think after releasing 2.1 IMHO. Kornel
That seems the right time then!
Currently, this is a patchset of 21 commits
http://git.lyx.org/?p=developers/tommaso/lyx.git;a=shortlog;h=refs/heads/features/chat2
Guess Vincent might comment on whether these should be squashed into a
single commit or similar?
T.
It would be easier to review if you would push the branch to the
features.git repository (which is accessible to everyone).

Vincent
Tommaso Cucinotta
2014-05-04 19:48:33 UTC
Permalink
It would be easier to review if you would push the branch to the features.git repository (which is accessible to everyone).
Hi,

I also pushed the branch to features.git as branch "chat3".

Now I've got into this issue:

$ git checkout features/chat3
warning: refname 'features/chat3' is ambiguous.

as I guess it's now finding, in repo tommaso, the "features/chat3" branch, and also, in repo "features", the branch "chat3".

How to disambiguate in git cmd-line invocations ?

Thanks,

T.
Vincent van Ravesteijn
2014-05-04 20:22:50 UTC
Permalink
Post by Tommaso Cucinotta
It would be easier to review if you would push the branch to the features.git repository (which is accessible to everyone).
Hi,
I also pushed the branch to features.git as branch "chat3".
$ git checkout features/chat3
warning: refname 'features/chat3' is ambiguous.
as I guess it's now finding, in repo tommaso, the "features/chat3" branch, and also, in repo "features", the branch "chat3".
How to disambiguate in git cmd-line invocations ?
Thanks,
T.
First, I think it is unnecessary to make a new branch with an increased
number. You can just overwrite the existing features/chat branch.

To disambiguate the refs, you can use:

"tommaso/features/chat3" for the branch features/chat3 in remote tommaso,
and
"remotes/features/chat3" for the branch chat3 in the remote features.

Vincent
Vincent van Ravesteijn
2013-11-18 08:41:25 UTC
Permalink
Post by Kornel Benko
Post by Vincent van Ravesteijn
The errors will not be the result of a 'git checkout' command. It more
looks like a git pull. This is the reason, I don't advice to use git
pull.
Post by Vincent van Ravesteijn
If you would have done a 'git fetch', you will retrieve the new commits.
Then, you want to reset the features/chat2 branch to the new one tommasso
$ git checkout features/chat2
$ git reset --hard <name_of_remote>/features/chat2
This must work without problems.
Vincent
Sure Vincent. If one knows how.
For some days I was not looking at this branch.
#git reset --hard features/chat2
HEAD is now at 2993f12 whitespaces
#git fetch
You already had fetched the branch before, and git should have told you
then (by head):

<sha1> -> <sha1> (forced update).

This (forced update) tells you that the branch has rewritten part of the
history, and you should either:
- rebase your commits on top of the remote branch, or
- reset your branch to the remote branch.

If you now merge with the remote branch, you will have a lot of commits in
your history that have two different sha1s.
Post by Kornel Benko
#git status
# On branch features/chat2
# Your branch and 'origin/features/chat2' have diverged,
# and have 16 and 33 different commits each, respectively.
#
nothing to commit (working directory clean)
Again, the fact that the branches have been diverged tells you the same
thing as above.
Post by Kornel Benko
#git branch
* features/chat2
master
#git pull
Pulling is not a good idea if the branched are diverged / rewritten,
because this merges two branches with almost the same commits but that have
different sha1s.
Post by Kornel Benko
What the mockery ... Nothing I try helps here. I for sure did not commit
or change anything.
The resolution would be (assuming the remote that points to tommaso's repo
is called "tommaso"):

$ git checkout features/chat2
$ git rebase tommaso/features/chat2 (depending on the degree of changes in
the remote, this might auto-detect which commits were already in the new
remote branch and which are new commits).

or

$ git checkout features/chat2
$ git rebase --onto tommaso/features/chat2 HEAD~4 HEAD (this manually tells
git which commits were your own and should be rebased on top of the remote
branch).

or

$ git checkout features/chat2
$ git reset --hard tommaso/features/chat2 (this throws away your local
commits and changes)

Vincent
Loading...