Re: [PATCH] lyxknitr failed when /tmp on different fs
I'm confident enough with the updated lyxknitr.R, but we may still need to test it under Windows. Regards, Yihui -- Yihui Xie Phone: 515-294-2465 Web: http://yihui.name Department of Statistics, Iowa State University 2215 Snedecor Hall, Ames, IA On Fri, Sep 28, 2012 at 1:43 AM, Scott Kostyshak wrote: > On Thu, Sep 27, 2012 at 3:56 PM, Scott Kostyshak wrote: >> On Thu, Sep 27, 2012 at 1:40 PM, Yihui Xie wrote: >>> Hi Scott, >>> >>> Attached is my patch. Now lyxknitr.R does not move files at all; all >>> old tricks are gone, and the R script is much cleaner. I have tested >>> it under Ubuntu. >> >> It looks a lot cleaner indeed. >> >>> I'm not sure you can commit it to the git repository, so I cc JMarc. >>> >>> BTW, I also updated the homepage of the knitr package from >>> yihui.github.com/knitr to yihui.name/knitr >> >> Great, thanks a lot Yihui! I will test this later. > > Yihui, testing went well for me, both on the computer where I have an > encrypted home and on a different computer. Unfortunately, they are > both also Ubuntu. Do you think this needs further testing on > non-Ubuntu computers? > > I updated the homepage of the knitr package in the example file. The > updated diff is attached. > > Scott
Re: [PATCH] lyxknitr failed when /tmp on different fs
On Thu, Sep 27, 2012 at 3:56 PM, Scott Kostyshak wrote:
> On Thu, Sep 27, 2012 at 1:40 PM, Yihui Xie wrote:
>> Hi Scott,
>>
>> Attached is my patch. Now lyxknitr.R does not move files at all; all
>> old tricks are gone, and the R script is much cleaner. I have tested
>> it under Ubuntu.
>
> It looks a lot cleaner indeed.
>
>> I'm not sure you can commit it to the git repository, so I cc JMarc.
>>
>> BTW, I also updated the homepage of the knitr package from
>> yihui.github.com/knitr to yihui.name/knitr
>
> Great, thanks a lot Yihui! I will test this later.
Yihui, testing went well for me, both on the computer where I have an
encrypted home and on a different computer. Unfortunately, they are
both also Ubuntu. Do you think this needs further testing on
non-Ubuntu computers?
I updated the homepage of the knitr package in the example file. The
updated diff is attached.
Scott
diff --git a/lib/examples/knitr.lyx b/lib/examples/knitr.lyx
index ddbfe01..feab161 100644
--- a/lib/examples/knitr.lyx
+++ b/lib/examples/knitr.lyx
@@ -182,7 +182,7 @@ status collapsed
\begin_layout Plain Layout
-http://yihui.github.com/knitr
+http://yihui.name/knitr
\end_layout
\end_inset
diff --git a/lib/layouts/knitr.module b/lib/layouts/knitr.module
index 694ae77..fe64359 100644
--- a/lib/layouts/knitr.module
+++ b/lib/layouts/knitr.module
@@ -1,7 +1,7 @@
#\DeclareLyXModule[knitr->latex]{Rnw (knitr)}
#DescriptionBegin
#Uses the knitr package in R for dynamic report generation. This R package has
to be installed for this module to work: install.packages('knitr'). Note it
depends on R >= 2.14.1.
-#For more info see http://yihui.github.com/knitr
+#For more info see http://yihui.name/knitr
#DescriptionEnd
#Category: literate
#Excludes: lilypond | sweave
diff --git a/lib/scripts/lyxknitr.R b/lib/scripts/lyxknitr.R
index 03150f2..7c029ea 100644
--- a/lib/scripts/lyxknitr.R
+++ b/lib/scripts/lyxknitr.R
@@ -11,7 +11,7 @@
## author Yihui Xie
## knitr is an alternative package to Sweave, and has more features
-## and flexibility; see https://yihui.github.com/knitr
+## and flexibility; see https://yihui.name/knitr
## Rscript $$s/scripts/lyxknitr.R $$p$$i $$p$$o $$e $$r
## $$p the path of the output (temp dir)
@@ -31,18 +31,7 @@ options(encoding = .cmdargs[3])
## can put your data files there and functions like read.table() can
## work correctly without specifying the full path
setwd(.cmdargs[4])
+opts_knit$set(root.dir = getwd())
-## copy the Rnw file to the current working directory if it does not exist
-.tmp.file = tempfile(); .rnw.file = basename(.cmdargs[1])
-.rnw.exists = file.exists(.rnw.file)
-if (.rnw.exists) file.rename(.rnw.file, .tmp.file)
-file.copy(.cmdargs[1], '.')
## run knit() to get .tex or .R
-knit(.rnw.file, tangle = 'tangle' %in% .cmdargs)
-
-setwd(.cmdargs[4])
-## remove the copied .Rnw if it did not exist, otherwise move the original one
back
-if (.rnw.exists) file.rename(.tmp.file, .rnw.file) else unlink(.rnw.file)
-file.rename(basename(.cmdargs[2]), .cmdargs[2]) # move .tex to the temp dir
-rm(.tmp.file, .rnw.file, .rnw.exists) # clean up these variables
-
+knit(.cmdargs[1], output = .cmdargs[2], tangle = 'tangle' %in% .cmdargs)
Re: [PATCH] lyxknitr failed when /tmp on different fs
On Thu, Sep 27, 2012 at 1:40 PM, Yihui Xie wrote: > Hi Scott, > > Attached is my patch. Now lyxknitr.R does not move files at all; all > old tricks are gone, and the R script is much cleaner. I have tested > it under Ubuntu. It looks a lot cleaner indeed. > I'm not sure you can commit it to the git repository, so I cc JMarc. > > BTW, I also updated the homepage of the knitr package from > yihui.github.com/knitr to yihui.name/knitr Great, thanks a lot Yihui! I will test this later. Scott
Re: [patch] fix warning on branch
On Thu, Sep 27, 2012 at 1:51 PM, Julien Rioux
wrote:
> On 27/09/2012 7:17 AM, Scott Kostyshak wrote:
>>
>> - ::write(pipefd, cmd.c_str(), cmd.length());
>> +if (::write(pipefd, cmd.c_str(), cmd.length()) < 0)
>> +LYXERR0("Cannot write to pipe!");
>
>
> You're space-indenting in a tab-indented file here.
>
Good catch. I thought I had git set up to detect these white space
errors but I guess not.
Attached is the updated patch.
Thanks,
Scott
From 35dcd7f6fb88a47bda9342e53d528cb521f34505 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak
Date: Thu, 27 Sep 2012 06:29:27 -0400
Subject: [PATCH] Use a return value to report an unsuccessful write
Use a return value in LyXComm::loadFilesInOtherInstance to give an
error for a failed write to pipe.
---
src/Server.cpp |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/Server.cpp b/src/Server.cpp
index 7ec096e..1e673e6 100644
--- a/src/Server.cpp
+++ b/src/Server.cpp
@@ -1015,7 +1015,8 @@ bool LyXComm::loadFilesInOtherInstance()
break;
string const cmd = "LYXCMD:pipe:file-open:" +
fname.absFileName() + '\n';
- ::write(pipefd, cmd.c_str(), cmd.length());
+ if (::write(pipefd, cmd.c_str(), cmd.length()) < 0)
+ LYXERR0("Cannot write to pipe!");
::close(pipefd);
++loaded_files;
it = theFilesToLoad().erase(it);
--
1.7.9.5
Re: [LyX 2.0.x] Fix bug #8349: Cannot compile 2.0.x: unresolved external symbol
Op 25-9-2012 10:24, Jean-Marc Lasgouttes schreef: Le 24/09/12 19:33, Vincent van Ravesteijn a écrit : Op 24-9-2012 16:28, Jean-Marc Lasgouttes schreef: That's amusing. This is a patch from Stephan that I backported, and it looks like Stephan did the backport, which does not help communication. Just get used to a more advanced form of communication. It is Stephan's code, which you committed. We just need to fix the e-mail send script if you really want to Actually, I understand that. My point is to get the correct answer to "who pushed this thing?" You're probably interested in who committed the "thing". Check >> git show 8169347 --pretty=full -n1 commit 8169347 Author: Stephan Witt Commit: Jean-Marc Vincent
Re: [patch] fix warning on branch
On 27/09/2012 7:17 AM, Scott Kostyshak wrote:
- ::write(pipefd, cmd.c_str(), cmd.length());
+if (::write(pipefd, cmd.c_str(), cmd.length()) < 0)
+LYXERR0("Cannot write to pipe!");
You're space-indenting in a tab-indented file here.
--
Julien
Re: [PATCH] lyxknitr failed when /tmp on different fs
Hi Scott, Attached is my patch. Now lyxknitr.R does not move files at all; all old tricks are gone, and the R script is much cleaner. I have tested it under Ubuntu. I'm not sure you can commit it to the git repository, so I cc JMarc. BTW, I also updated the homepage of the knitr package from yihui.github.com/knitr to yihui.name/knitr Regards, Yihui -- Yihui Xie Phone: 515-294-2465 Web: http://yihui.name Department of Statistics, Iowa State University 2215 Snedecor Hall, Ames, IA On Thu, Sep 27, 2012 at 6:18 AM, Scott Kostyshak wrote: > When /tmp was on a different file system (e.g. encrypted home), > lyxknitr.R failed to move files to /tmp because it relied on R's > 'file.rename' function, which in turn relied on the rename function in > , which was failing with the EXDEV errno. Now lyxknitr.R relies > on 'file.copy'. > > Yihui, does this look OK? > > Note that I also changed a comment: file.rename renamed a .tex and a .R for > me. > > Thanks, > > Scott lyxknitr.diff Description: Binary data
Re: [PATCH] lyxknitr failed when /tmp on different fs
I was bitten by this problem as well a few days ago, but I forgot to work on it. I think I have a better fix which does not involve with copying or renaming files at all. I'll do it soon. Thanks! Regards, Yihui -- Yihui Xie Phone: 515-294-2465 Web: http://yihui.name Department of Statistics, Iowa State University 2215 Snedecor Hall, Ames, IA On Thu, Sep 27, 2012 at 6:18 AM, Scott Kostyshak wrote: > When /tmp was on a different file system (e.g. encrypted home), > lyxknitr.R failed to move files to /tmp because it relied on R's > 'file.rename' function, which in turn relied on the rename function in > , which was failing with the EXDEV errno. Now lyxknitr.R relies > on 'file.copy'. > > Yihui, does this look OK? > > Note that I also changed a comment: file.rename renamed a .tex and a .R for > me. > > Thanks, > > Scott
Re: [patch] fix warning on branch
Le 27/09/2012 13:17, Scott Kostyshak a écrit : OK. I'm trying to stay away from branch as much as possible for now. But attached are the three patches. Can someone check them and make sure they are risk free? They seem OK to me. I am not sure what the lexer code does, but the patch does not change that anyway. JMarc
Re: [PATCH] lyxknitr failed when /tmp on different fs
Am Donnerstag, 27. September 2012 um 07:18:36, schrieb Scott Kostyshak > When /tmp was on a different file system (e.g. encrypted home), > lyxknitr.R failed to move files to /tmp because it relied on R's > 'file.rename' function, which in turn relied on the rename function in > , which was failing with the EXDEV errno. Now lyxknitr.R relies > on 'file.copy'. > > Yihui, does this look OK? > > Note that I also changed a comment: file.rename renamed a .tex and a .R for > me. > > Thanks, > > Scott Rename does not change modify time. Copy does. Don't know, if that may cause problems. Kornel signature.asc Description: This is a digitally signed message part.
[PATCH] lyxknitr failed when /tmp on different fs
When /tmp was on a different file system (e.g. encrypted home), lyxknitr.R failed to move files to /tmp because it relied on R's 'file.rename' function, which in turn relied on the rename function in , which was failing with the EXDEV errno. Now lyxknitr.R relies on 'file.copy'. Yihui, does this look OK? Note that I also changed a comment: file.rename renamed a .tex and a .R for me. Thanks, Scott 0001-lyxknitr.R-failed-when-tmp-on-different-fs.patch Description: Binary data
Re: [patch] fix warning on branch
On Mon, Sep 24, 2012 at 1:04 PM, Richard Heck wrote:
> On 09/23/2012 04:54 PM, Jean-Marc Lasgouttes wrote:
>>
>> Le 23/09/12 00:06, Scott Kostyshak a écrit :
How many users *compile* anything :)
>>>
>>>
>>> True. And out of those who do, how many look at the warnings? :)
>>
>>
>> Personally, I still think that removing warnings in branch matters.
>>
> Yes, I think it's worth doing, but not if it's the kind of thing that has
> any
> chance of causing problems. I think that was the issue.
OK. I'm trying to stay away from branch as much as possible for now.
But attached are the three patches. Can someone check them and make
sure they are risk free?
Thanks,
Scott
From 877398ce2faa5751a62639c9c20978eb3ff33c35 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak
Date: Sun, 23 Sep 2012 04:58:19 -0400
Subject: [PATCH 1/3] Remove unused function find_matching_brace
find_matching_brace came over from a backport and is not being used.
---
src/lyxfind.cpp | 21 -
1 file changed, 21 deletions(-)
diff --git a/src/lyxfind.cpp b/src/lyxfind.cpp
index f2c941b..a103d76 100644
--- a/src/lyxfind.cpp
+++ b/src/lyxfind.cpp
@@ -562,27 +562,6 @@ string apply_escapes(string s, Escapes const & escape_map)
return s;
}
-/** Return the position of the closing brace matching the open one at s[pos],
- ** or s.size() if not found.
- **/
-static size_t find_matching_brace(string const & s, size_t pos)
-{
- LASSERT(s[pos] == '{', /* */);
- int open_braces = 1;
- for (++pos; pos < s.size(); ++pos) {
- if (s[pos] == '\\')
- ++pos;
- else if (s[pos] == '{')
- ++open_braces;
- else if (s[pos] == '}') {
- --open_braces;
- if (open_braces == 0)
- return pos;
- }
- }
- return s.size();
-}
-
/// Within \regexp{} apply get_lyx_unescapes() only (i.e., preserve regexp
semantics of the string),
/// while outside apply get_lyx_unescapes()+get_regexp_escapes().
/// If match_latex is true, then apply regexp_latex_escapes() to \regexp{}
contents as well.
--
1.7.9.5
From b7582fc12ac0b8130ff26f7a735b12e200e5a714 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak
Date: Thu, 27 Sep 2012 06:26:46 -0400
Subject: [PATCH 2/3] Get rid of unused variable to eliminate warning
The variable 'c' in Lexer::Pimpl::setFile was not being used.
---
src/Lexer.cpp |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Lexer.cpp b/src/Lexer.cpp
index d824da0..664e1b9 100644
--- a/src/Lexer.cpp
+++ b/src/Lexer.cpp
@@ -274,9 +274,9 @@ bool Lexer::Pimpl::setFile(FileName const & filename)
// Skip byte order mark.
if (is.peek() == 0xef) {
- int c = is.get();
+ is.get();
if (is.peek() == 0xbb) {
- c = is.get();
+ is.get();
LASSERT(is.get() == 0xbf, /**/);
} else
is.unget();
--
1.7.9.5
From 4dfa0d7641b40eede22dd2da19da4ff83ef6a470 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak
Date: Thu, 27 Sep 2012 06:29:27 -0400
Subject: [PATCH 3/3] Use a return value to report an unsuccessful write
Use a return value in LyXComm::loadFilesInOtherInstance to give an
error for a failed write to pipe.
---
src/Server.cpp |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/Server.cpp b/src/Server.cpp
index 7ec096e..09a8734 100644
--- a/src/Server.cpp
+++ b/src/Server.cpp
@@ -1015,7 +1015,8 @@ bool LyXComm::loadFilesInOtherInstance()
break;
string const cmd = "LYXCMD:pipe:file-open:" +
fname.absFileName() + '\n';
- ::write(pipefd, cmd.c_str(), cmd.length());
+if (::write(pipefd, cmd.c_str(), cmd.length()) < 0)
+LYXERR0("Cannot write to pipe!");
::close(pipefd);
++loaded_files;
it = theFilesToLoad().erase(it);
--
1.7.9.5
