Re: Bug #11484
On Sat, Apr 24, 2021 at 08:21:27AM +0200, Jürgen Spitzmüller wrote: > Am Freitag, dem 23.04.2021 um 15:21 -0400 schrieb Scott Kostyshak: > > I do not think the new GUI behavior is good > > Proceed as you see fit. Fine with me. It would be nice to hear from Jürgen Womser-Schütz on their thoughts. Jürgen (Womser-Schütz), what is your specific use case? Is using a branch a reasonable way to achieve a workflow that addresses it? If not, would creating empty .lyx files for the child documents work? I imagine it is not ideal, but as you can see in this thread I am stubborn about the difference between a warning and an error. Nonetheless, I hope we can find something that addresses your workflow. Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
Am Freitag, dem 23.04.2021 um 15:21 -0400 schrieb Scott Kostyshak: > I do not think the new GUI behavior is good Proceed as you see fit. Fine with me. Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
On Fri, Apr 23, 2021 at 06:58:16AM +0200, Jürgen Spitzmüller wrote: > Am Donnerstag, dem 22.04.2021 um 13:18 -0400 schrieb Scott Kostyshak: > > I wonder if using branches would be a better way to achieve the > > desired workflow above. I believe we should preserve the previous > > default behavior of giving an error in the GUI when there is a > > missing included file. > > But only since warnings are not salient enough on the command line, > right? If so this fix is at the wrong place. I think the command line issue is separate from the GUI issue. Sorry for conflating the two in my initial email. We can always adapt the command-line behavior from what I understand (as the patch you just made suggests). I started this discussion with the command-line motivation because that is how I encountered it, but I do not think the new GUI behavior is good. I do not think the new GUI behavior is good because (a) I think the distinction between an error and a warning is important and (b) I think that a missing file should be an error. I guess this is a disagreement partly in semantics, which I wonder is difficult to have a constructive debate. But we can try. Should we do the same thing for included graphics that we can't find? What about a .bib file that does not exist? Perhaps a distinction can be made for why we do something special for an included child document and not other files, but to me the line seems blurry. Regarding workflow, I propose three ways to achieve the workflow you mentioned as motivation for the patch that I think are better than what the patch implemented: 1. Use branches. 2. Just create empty .lyx files for the includes. 3. Proceed with compilation and give an error if there is a missing include but the user can click on the "Show Output Anyway" button to see the file that omits the missing include. (3) is not currently achievable in LyX but presumably we could consider this enhancement. > > A couple of notes on issues related to the above. I think that they > > could be worked around if we decide that a warning is better than an > > error: > > > > 1. I think that LyX should try its best to export the same LaTeX code > > on > > different systems. I understand that in some cases it is difficult > > to > > avoid different exported code on different systems (I think we > > sometimes condition on which features are available?), but it > > would > > be nice to keep those cases to a minimum. After this commit, if > > the > > file exists on one system but does not exist on a different > > system, > > LyX exports different LaTeX code. Perhaps as an alternative LyX > > could > > add code to the preamble to redefine the \include command for > > LaTeX > > to check if the file exists and condition accordingly. I really > > don't > > like this idea but at least the same .tex code would be exported > > in > > all cases. > > > Thus the warning to indicate that something is unusual. I think an > error is much too brute here, as it disallows people to work with this > file. I think that the workflows proposed above allow people to work as desired. > > 2. Starting with this commit, if you view the source code (e.g., set > > to > > "Complete Source"), every time you type a character you get the > > warning. > > This also can be addressed at the core I think. Agreed. This is not a major concern. I only mentioned it in case we decide to keep the new behavior in c41df5b671e. Scott > > > Can we have warnings on the cl that require user prompts before the > > > process continues? > > > > Perhaps we could try to emulate what LaTeX does. They have prompts > > for > > undefined commands and if you press return the compilation tries to > > continue. They also have a batch mode where there is no prompt and it > > just errors out. I don't really like this idea of adding this > > prompt-like behavior to LyX's command-line export and an additional > > batch mode (preference?), but perhaps we could consider it if that's > > what the majority prefers. In any case, we might want to figure this > > out > > after we figure out the GUI behavior discussed above. > > Note that I mean this as an option for warning which we think require > attention (but are not errors). We could add an option > "cl_confirmation" to alert::warning, defaulting to false and enabled > for this particular warning. > > Jürgen > > > > > Scott > signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
Am Freitag, dem 23.04.2021 um 06:58 +0200 schrieb Jürgen Spitzmüller:
> Note that I mean this as an option for warning which we think require
> attention (but are not errors). We could add an option
> "cl_confirmation" to alert::warning, defaulting to false and enabled
> for this particular warning.
Like the attached.
Jürgen
diff --git a/src/frontends/alert.h b/src/frontends/alert.h
index 20da43656a..a7d5bfe2f5 100644
--- a/src/frontends/alert.h
+++ b/src/frontends/alert.h
@@ -41,11 +41,12 @@ int prompt(docstring const & title, docstring const & question,
* Only use this if the user cannot perform some remedial action.
* \p askshowagain will display a check box where the user can turn off the
* warning for future cases. Ponder carefully if this is feasible.
+ * \p clconfirm will ask for user confirmation on command line.
*
* The console output takes care of converting any Qt html to plain text.
*/
void warning(docstring const & title, docstring const & message,
- bool askshowagain = false);
+ bool askshowagain = false, bool clconfirm = false);
/**
* Display a warning to the user. Title should be a short (general) summary.
diff --git a/src/frontends/qt/GuiAlert.cpp b/src/frontends/qt/GuiAlert.cpp
index 93b8e1d4e6..cc806b40d2 100644
--- a/src/frontends/qt/GuiAlert.cpp
+++ b/src/frontends/qt/GuiAlert.cpp
@@ -150,14 +150,19 @@ int prompt(docstring const & title, docstring const & question,
}
void doWarning(docstring const & title, docstring const & message,
- bool askshowagain)
+ bool askshowagain, bool clconfirm)
{
lyxerr << "Warning: " << toPlainText(title) << '\n'
<< "\n"
<< toPlainText(message) << endl;
- if (!use_gui)
+ if (!use_gui) {
+ if (clconfirm) {
+ lyxerr << _("Press to continue");
+ getchar();
+ }
return;
+ }
if (theApp() == 0) {
noAppDialog(toqstr(title), toqstr(message), QMessageBox::Warning);
@@ -190,14 +195,14 @@ void doWarning(docstring const & title, docstring const & message,
}
void warning(docstring const & title, docstring const & message,
- bool askshowagain)
+ bool askshowagain, bool clconfirm)
{
#ifdef EXPORT_in_THREAD
InGuiThread().call(,
#else
doWarning(
#endif
-title, message, askshowagain);
+title, message, askshowagain, clconfirm);
}
void doError(docstring const & title, docstring const & message, bool backtrace)
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
index aeae2fb17d..647b11739f 100644
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -555,7 +555,7 @@ void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
"'%1$s'\n"
"has not been found. LyX will ignore the inclusion."),
from_utf8(incfile)),
- true);
+ true, true);
return;
}
diff --git a/src/tests/check_layout.cpp b/src/tests/check_layout.cpp
index afdb66777d..23099bb07a 100644
--- a/src/tests/check_layout.cpp
+++ b/src/tests/check_layout.cpp
@@ -21,7 +21,7 @@ using namespace std;
namespace lyx {
namespace frontend {
namespace Alert {
-void warning(docstring const & title, docstring const & message, bool)
+void warning(docstring const & title, docstring const & message, bool, bool)
{
LYXERR0(title);
LYXERR0(message);
diff --git a/src/tex2lyx/dummy_impl.cpp b/src/tex2lyx/dummy_impl.cpp
index 961eb9ae0d..776cc28587 100644
--- a/src/tex2lyx/dummy_impl.cpp
+++ b/src/tex2lyx/dummy_impl.cpp
@@ -38,7 +38,7 @@ namespace lyx {
namespace frontend {
namespace Alert {
- void warning(docstring const & title, docstring const & message, bool)
+ void warning(docstring const & title, docstring const & message, bool, bool)
{
cerr << to_utf8(title) << "\n" << to_utf8(message) << endl;
}
signature.asc
Description: This is a digitally signed message part
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
Am Donnerstag, dem 22.04.2021 um 13:18 -0400 schrieb Scott Kostyshak: > I wonder if using branches would be a better way to achieve the > desired workflow above. I believe we should preserve the previous > default behavior of giving an error in the GUI when there is a > missing included file. But only since warnings are not salient enough on the command line, right? If so this fix is at the wrong place. > A couple of notes on issues related to the above. I think that they > could be worked around if we decide that a warning is better than an > error: > > 1. I think that LyX should try its best to export the same LaTeX code > on > different systems. I understand that in some cases it is difficult > to > avoid different exported code on different systems (I think we > sometimes condition on which features are available?), but it > would > be nice to keep those cases to a minimum. After this commit, if > the > file exists on one system but does not exist on a different > system, > LyX exports different LaTeX code. Perhaps as an alternative LyX > could > add code to the preamble to redefine the \include command for > LaTeX > to check if the file exists and condition accordingly. I really > don't > like this idea but at least the same .tex code would be exported > in > all cases. Thus the warning to indicate that something is unusual. I think an error is much too brute here, as it disallows people to work with this file. > 2. Starting with this commit, if you view the source code (e.g., set > to > "Complete Source"), every time you type a character you get the > warning. This also can be addressed at the core I think. > > > Can we have warnings on the cl that require user prompts before the > > process continues? > > Perhaps we could try to emulate what LaTeX does. They have prompts > for > undefined commands and if you press return the compilation tries to > continue. They also have a batch mode where there is no prompt and it > just errors out. I don't really like this idea of adding this > prompt-like behavior to LyX's command-line export and an additional > batch mode (preference?), but perhaps we could consider it if that's > what the majority prefers. In any case, we might want to figure this > out > after we figure out the GUI behavior discussed above. Note that I mean this as an option for warning which we think require attention (but are not errors). We could add an option "cl_confirmation" to alert::warning, defaulting to false and enabled for this particular warning. Jürgen > > Scott signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
On Sun, Apr 18, 2021 at 09:14:17AM +0200, Jürgen Spitzmüller wrote: > Am Freitag, dem 16.04.2021 um 12:43 -0400 schrieb Scott Kostyshak: > > Thanks to the Jürgens for your work on this. On master branch, if I > > export a file on the command-line with a child document that is not > > found, I no longer get an error and the command exits "successfully" > > (i.e., 0 exit code). It shows a warning in the terminal but it is > > easy > > to miss it. > > > > I attach a patch that amends c41df5b671e by converting the warning to > > an > > error. Any objection? If preferred, we could show both the warning > > and > > the error, but it seems reasonable to show just one message of the > > problem. > > The advantage of the warning is that you can still productively work > with the file (e.g. if you don't have the included file at hand, but > want to edit the master only anyway; or if the included file is still > to be written). I wonder if using branches would be a better way to achieve the desired workflow above. I believe we should preserve the previous default behavior of giving an error in the GUI when there is a missing included file. A couple of notes on issues related to the above. I think that they could be worked around if we decide that a warning is better than an error: 1. I think that LyX should try its best to export the same LaTeX code on different systems. I understand that in some cases it is difficult to avoid different exported code on different systems (I think we sometimes condition on which features are available?), but it would be nice to keep those cases to a minimum. After this commit, if the file exists on one system but does not exist on a different system, LyX exports different LaTeX code. Perhaps as an alternative LyX could add code to the preamble to redefine the \include command for LaTeX to check if the file exists and condition accordingly. I really don't like this idea but at least the same .tex code would be exported in all cases. 2. Starting with this commit, if you view the source code (e.g., set to "Complete Source"), every time you type a character you get the warning. > Can we have warnings on the cl that require user prompts before the > process continues? Perhaps we could try to emulate what LaTeX does. They have prompts for undefined commands and if you press return the compilation tries to continue. They also have a batch mode where there is no prompt and it just errors out. I don't really like this idea of adding this prompt-like behavior to LyX's command-line export and an additional batch mode (preference?), but perhaps we could consider it if that's what the majority prefers. In any case, we might want to figure this out after we figure out the GUI behavior discussed above. Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
Am Freitag, dem 16.04.2021 um 12:43 -0400 schrieb Scott Kostyshak: > Thanks to the Jürgens for your work on this. On master branch, if I > export a file on the command-line with a child document that is not > found, I no longer get an error and the command exits "successfully" > (i.e., 0 exit code). It shows a warning in the terminal but it is > easy > to miss it. > > I attach a patch that amends c41df5b671e by converting the warning to > an > error. Any objection? If preferred, we could show both the warning > and > the error, but it seems reasonable to show just one message of the > problem. The advantage of the warning is that you can still productively work with the file (e.g. if you don't have the included file at hand, but want to edit the master only anyway; or if the included file is still to be written). Can we have warnings on the cl that require user prompts before the process continues? Jürgen signature.asc Description: This is a digitally signed message part -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
On Tue, Mar 12, 2019 at 09:02:22AM +0100, Jürgen Spitzmüller wrote:
> Am Montag, den 11.03.2019, 18:59 +0100 schrieb Jürgen Womser-Schütz:
> > Hello Jürgen,
> >
> > I think it is now much better than I could do it without your help.
Thanks to the Jürgens for your work on this. On master branch, if I
export a file on the command-line with a child document that is not
found, I no longer get an error and the command exits "successfully"
(i.e., 0 exit code). It shows a warning in the terminal but it is easy
to miss it.
I attach a patch that amends c41df5b671e by converting the warning to an
error. Any objection? If preferred, we could show both the warning and
the error, but it seems reasonable to show just one message of the
problem.
I also attach an example .lyx file in case someone wants to experiment.
I intentionally do not attach the child .lyx file.
Scott
From 485e80aa4bb1dc1f1036a460cab5f3e11d69deff Mon Sep 17 00:00:00 2001
From: Scott Kostyshak
Date: Fri, 16 Apr 2021 11:55:43 -0400
Subject: [PATCH] If file not found, give error instead of warn
c41df5b671e introduced a "warn and return" if an included file does
not exist. On the command line, an export now returns with a 0 exit
code in this case and thus a missing child can go undetected if
relying on command-line export.
Before c41df5b671e, the code would in some cases at least reach a
separate error, "Could not load included file"; however, the return
caused this error to not be reached.
---
src/insets/InsetInclude.cpp | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
index aeae2fb17d..907ead0d5f 100644
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -548,15 +548,18 @@ void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
true);
return;
}
- // Warn if file doesn't exist
+
+ // Give error if file doesn't exist. Without this error, an error is
+ // triggered in some cases by our check of the return value of
+ // loadIfNeeded() below, but not others so we give an error now to be
+ // sure.
if (!includedFileExist()) {
- frontend::Alert::warning(_("Included file not found"),
- bformat(_("The included file\n"
- "'%1$s'\n"
- "has not been found. LyX will ignore the inclusion."),
-from_utf8(incfile)),
- true);
- return;
+ docstring text = bformat(_("Included file not found. "
+ "File\n`%1$s'\n"
+ "has not been found."),
+from_utf8(incfile));
+ throw ExceptionMessage(ErrorException, _("Error: "),
+ text);
}
FileName const included_file = includedFileName(buffer(), params());
--
2.20.1
example.lyx
Description: application/lyx
signature.asc
Description: PGP signature
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Bug #11484
Am Montag, den 11.03.2019, 18:59 +0100 schrieb Jürgen Womser-Schütz: > Hello Jürgen, > > I think it is now much better than I could do it without your help. We are a collaborative project, after all. > But > I am still learning :-) So am I. > Another thing: > In the case of include type "Input" or "Include" only LyX or LaTeX- > files > are meaningful. > I wasn't aware this fact during my tests and so I got conversion > errors > because I was using a c-file instead of *.lyx/*.tex. > > I know in Linux istn't a strong binding between content and > fileextension usual. > But do you think the dialog should give users a hint not to select a > unappropriate file? If "Yes" i could try to improve this. There are contexts where inserting something else makes sense. For instance PGF graphics (*.tikz, *.pgf). metapost drawings (*.mp). Even a c-file can make sense in specific environments. So I don't think we should restrict the selection. I have committed now what we have. We can always improve things in subsequent commits. Riki, this could also be backported. Looking forward to your forthcoming patches. In any case, you have been added to the Hall of Fame: https://www.lyx.org/Credits Jürgen signature.asc Description: This is a digitally signed message part
Re: Bug #11484
Hello Jürgen, I think it is now much better than I could do it without your help. But I am still learning :-) Another thing: In the case of include type "Input" or "Include" only LyX or LaTeX-files are meaningful. I wasn't aware this fact during my tests and so I got conversion errors because I was using a c-file instead of *.lyx/*.tex. I know in Linux istn't a strong binding between content and fileextension usual. But do you think the dialog should give users a hint not to select a unappropriate file? If "Yes" i could try to improve this. Jürgen (JWS) Am 11.03.2019 um 13:11 schrieb Jürgen Spitzmüller: > Am Mo., 11. März 2019 um 08:40 Uhr schrieb Jürgen Spitzmüller > > Some more comments: > > > The attached patch includes my suggestions (incl. the ltrimming of > the file name). > > What do you think, Jürgen? > > Jürgen > (not talking to himself) >
Re: Bug #11484
Am Mo., 11. März 2019 um 08:40 Uhr schrieb Jürgen Spitzmüller
> Some more comments:
>
The attached patch includes my suggestions (incl. the ltrimming of the
file name).
What do you think, Jürgen?
Jürgen
(not talking to himself)
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
old mode 100644
new mode 100755
index 51cc147333..422b711729
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -55,6 +55,7 @@
#include "support/convert.h"
#include "support/debug.h"
#include "support/docstream.h"
+#include "support/FileName.h"
#include "support/FileNameList.h"
#include "support/filetools.h"
#include "support/gettext.h"
@@ -151,7 +152,7 @@ string const parentFileName(Buffer const & buffer)
FileName const includedFileName(Buffer const & buffer,
InsetCommandParams const & params)
{
- return makeAbsPath(to_utf8(params["filename"]),
+ return makeAbsPath(ltrim(to_utf8(params["filename"])),
onlyPath(parentFileName(buffer)));
}
@@ -188,7 +189,7 @@ char_type replaceCommaInBraces(docstring & params)
InsetInclude::InsetInclude(Buffer * buf, InsetCommandParams const & p)
: InsetCommand(buf, p), include_label(uniqueID()),
preview_(make_unique(this)), failedtoload_(false),
- set_label_(false), label_(0), child_buffer_(0)
+ set_label_(false), label_(0), child_buffer_(0), file_exist_(false)
{
preview_->connect([=](){ fileChanged(); });
@@ -203,7 +204,7 @@ InsetInclude::InsetInclude(Buffer * buf, InsetCommandParams const & p)
InsetInclude::InsetInclude(InsetInclude const & other)
: InsetCommand(other), include_label(other.include_label),
preview_(make_unique(this)), failedtoload_(false),
- set_label_(false), label_(0), child_buffer_(0)
+ set_label_(false), label_(0), child_buffer_(0), file_exist_(other.file_exist_)
{
preview_->connect([=](){ fileChanged(); });
@@ -265,7 +266,7 @@ void InsetInclude::doDispatch(Cursor & cur, FuncRequest & cmd)
switch (cmd.action()) {
case LFUN_INSET_EDIT: {
- editIncluded(to_utf8(params()["filename"]));
+ editIncluded(ltrim(to_utf8(params()["filename"])));
break;
}
@@ -385,12 +386,14 @@ bool InsetInclude::isChildIncluded() const
return true;
return (std::find(includeonlys.begin(),
includeonlys.end(),
- to_utf8(params()["filename"])) != includeonlys.end());
+ ltrim(to_utf8(params()["filename"]))) != includeonlys.end());
}
docstring InsetInclude::screenLabel() const
{
+ docstring pre = file_exist_ ? docstring() : _("FILE MISSING:");
+
docstring temp;
switch (type(params())) {
@@ -419,12 +422,12 @@ docstring InsetInclude::screenLabel() const
temp += ": ";
- if (params()["filename"].empty())
+ if (ltrim(params()["filename"]).empty())
temp += "???";
else
- temp += from_utf8(onlyFileName(to_utf8(params()["filename"])));
+ temp += from_utf8(onlyFileName(ltrim(to_utf8(params()["filename"];
- return temp;
+ return pre.empty() ? temp : pre + from_ascii(" ") + temp;
}
@@ -511,11 +514,26 @@ Buffer * InsetInclude::loadIfNeeded() const
void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
{
- string incfile = to_utf8(params()["filename"]);
-
- // Do nothing if no file name has been specified
- if (incfile.empty())
+ string incfile = ltrim(to_utf8(params()["filename"]));
+
+ // Warn if no file name has been specified
+ if (incfile.empty()) {
+ frontend::Alert::warning(_("No file name specified"),
+ _("An included file name is empty.\n"
+ "Ignoring Inclusion"),
+ true);
return;
+ }
+ // Warn if file doesn't exist
+ if (!includedFileExist()) {
+ frontend::Alert::warning(_("Included file not found"),
+ bformat(_("The included file\n"
+ "'%1$s'\n"
+ "has not been found. LyX will ignore the inclusion."),
+from_utf8(incfile)),
+ true);
+ return;
+ }
FileName const included_file = includedFileName(buffer(), params());
@@ -916,7 +934,7 @@ docstring InsetInclude::xhtml(XHTMLStream & xs, OutputParams const & rp) const
frontend::Alert::warning(_("Unsupported Inclusion"),
bformat(_("LyX does not know how to include non-LyX files when "
"generating HTML output. Offending file:\n%1$s"),
- params()["filename"]));
+ ltrim(params()["filename"])));
return docstring();
}
@@ -927,7 +945,7 @@ docstring InsetInclude::xhtml(XHTMLStream & xs, OutputParams const & rp) const
if (buffer().absFileName() == included_file.absFileName()) {
Alert::error(_("Recursive input"),
bformat(_("Attempted to include file %1$s in itself! "
- "Ignoring inclusion."), params()["filename"]));
+ "Ignoring inclusion."), ltrim(params()["filename"])));
return docstring();
}
@@ -962,7 +980,7 @@ int InsetInclude::plaintext(odocstringstream & os,
// or are generating this for advanced search
if (op.for_tooltip || op.for_toc || op.for_search) {
os << '[' << screenLabel() << '\n'
- << getParam("filename") <<
Re: Bug #11484
Am Mo., 11. März 2019 um 08:40 Uhr schrieb Jürgen Spitzmüller:
> BTW I see that the file name is not trimmed (hence " " is not treated as
> empty filename); we should probably do that:
>
> if (trim(params()["filename"]).empty())
> temp += "???";
>
It turns out we need to ltrim the file name in all uses of the filename
param. Otherwise, such files do not compile (rtrim is not necessary).
See attached patch.
Jürgen
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
index 51cc147333..29463863c7 100644
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -151,7 +151,7 @@ string const parentFileName(Buffer const & buffer)
FileName const includedFileName(Buffer const & buffer,
InsetCommandParams const & params)
{
- return makeAbsPath(to_utf8(params["filename"]),
+ return makeAbsPath(ltrim(to_utf8(params["filename"])),
onlyPath(parentFileName(buffer)));
}
@@ -265,7 +265,7 @@ void InsetInclude::doDispatch(Cursor & cur, FuncRequest & cmd)
switch (cmd.action()) {
case LFUN_INSET_EDIT: {
- editIncluded(to_utf8(params()["filename"]));
+ editIncluded(ltrim(to_utf8(params()["filename"])));
break;
}
@@ -385,7 +385,7 @@ bool InsetInclude::isChildIncluded() const
return true;
return (std::find(includeonlys.begin(),
includeonlys.end(),
- to_utf8(params()["filename"])) != includeonlys.end());
+ ltrim(to_utf8(params()["filename"]))) != includeonlys.end());
}
@@ -419,10 +419,10 @@ docstring InsetInclude::screenLabel() const
temp += ": ";
- if (params()["filename"].empty())
+ if (ltrim(params()["filename"]).empty())
temp += "???";
else
- temp += from_utf8(onlyFileName(to_utf8(params()["filename"])));
+ temp += from_utf8(onlyFileName(ltrim(to_utf8(params()["filename"];
return temp;
}
@@ -511,7 +511,7 @@ Buffer * InsetInclude::loadIfNeeded() const
void InsetInclude::latex(otexstream & os, OutputParams const & runparams) const
{
- string incfile = to_utf8(params()["filename"]);
+ string incfile = ltrim(to_utf8(params()["filename"]));
// Do nothing if no file name has been specified
if (incfile.empty())
@@ -916,7 +916,7 @@ docstring InsetInclude::xhtml(XHTMLStream & xs, OutputParams const & rp) const
frontend::Alert::warning(_("Unsupported Inclusion"),
bformat(_("LyX does not know how to include non-LyX files when "
"generating HTML output. Offending file:\n%1$s"),
- params()["filename"]));
+ ltrim(params()["filename"])));
return docstring();
}
@@ -927,7 +927,7 @@ docstring InsetInclude::xhtml(XHTMLStream & xs, OutputParams const & rp) const
if (buffer().absFileName() == included_file.absFileName()) {
Alert::error(_("Recursive input"),
bformat(_("Attempted to include file %1$s in itself! "
- "Ignoring inclusion."), params()["filename"]));
+ "Ignoring inclusion."), ltrim(params()["filename"])));
return docstring();
}
@@ -962,7 +962,7 @@ int InsetInclude::plaintext(odocstringstream & os,
// or are generating this for advanced search
if (op.for_tooltip || op.for_toc || op.for_search) {
os << '[' << screenLabel() << '\n'
- << getParam("filename") << "\n]";
+ << ltrim(getParam("filename")) << "\n]";
return PLAINTEXT_NEWLINE + 1; // one char on a separate line
}
@@ -992,7 +992,7 @@ int InsetInclude::plaintext(odocstringstream & os,
int InsetInclude::docbook(odocstream & os, OutputParams const & runparams) const
{
- string incfile = to_utf8(params()["filename"]);
+ string incfile = ltrim(to_utf8(params()["filename"]));
// Do nothing if no file name has been specified
if (incfile.empty())
@@ -1056,7 +1056,7 @@ void InsetInclude::validate(LaTeXFeatures & features) const
{
LATTEST(() == ());
- string incfile = to_utf8(params()["filename"]);
+ string incfile = ltrim(to_utf8(params()["filename"]));
string const included_file =
includedFileName(buffer(), params()).absFileName();
Re: Bug #11484
Am So., 10. März 2019 um 21:56 Uhr schrieb Jürgen Womser-Schütz <
jws1...@gmx.de>:
> Jürgen's improvements for "GNU Gettext" are now includet.
> I looked into the documentation (chapter "Child Documents" in the
> "Embedded Objects" helpfile ): a change seems to me not really necessary.
> Maybe it is self explanatory enough.
>
Thanks. Some more comments:
-if (params()["filename"].empty())
> -temp += "???";
> -else
> -temp += from_utf8(onlyFileName(to_utf8(params()["filename"])));
>
Doesn't it make sense to print those question marks if the filename is
empty? I know you cannot enter such a thing via the dialog, but you can via
"inset-insert include".
BTW I see that the file name is not trimmed (hence " " is not treated as
empty filename); we should probably do that:
if (trim(params()["filename"]).empty())
temp += "???";
> -// Do nothing if no file name has been specified
> -if (incfile.empty())
> +// Do nothing if file doesn't exist
> +if (!listingFileExist())
> return;
>
One thing we can do if the file doesn't exist (and maybe also if we have an
empty filename) is to issue an opt-out warning:
// Warn if no file name has been specified
if (incfile.empty()) {
frontend::Alert::warning(_("No file name specified"),
_("An included file name is empty.\n"
"Ignoring Inclusion"),
true);
return;
}
// Warn if file doesn't exist
if (!listingFileExist()) {
frontend::Alert::warning(_("Included file not found"),
bformat(_("The included file\n"
"'%1$s'\n"
"has not been found. LyX will ignore the
inclusion."), from_utf8(incfile)),
true);
return;
}
> void InsetInclude::updateBuffer(ParIterator const & it, UpdateType utype)
> {
> +file_exist_ = listingFileExist();
>
This method does not only check for listings files, does it? So I would
rename it to something more general
(includedFileExists())
Jürgen
Re: Bug #11484
Hi,
Jürgen's improvements for "GNU Gettext" are now includet.
I looked into the documentation (chapter "Child Documents" in the
"Embedded Objects" helpfile ): a change seems to me not really necessary.
Maybe it is self explanatory enough.
Jürgen
Am 10.03.2019 um 19:17 schrieb Jürgen Spitzmüller:
> Am Sonntag, den 10.03.2019, 19:08 +0100 schrieb Jürgen Womser-Schütz:
>> Ahh, I see: _() is the C++ user defined flag for enabling the
>> translation machinery! I couldn't find the definition :-(
> src/support/getttext.h
>
> Jürgen
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
old mode 100644
new mode 100755
index 51cc147333..f77dad2b03
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -55,6 +55,7 @@
#include "support/convert.h"
#include "support/debug.h"
#include "support/docstream.h"
+#include "support/FileName.h"
#include "support/FileNameList.h"
#include "support/filetools.h"
#include "support/gettext.h"
@@ -188,7 +189,7 @@ char_type replaceCommaInBraces(docstring & params)
InsetInclude::InsetInclude(Buffer * buf, InsetCommandParams const & p)
: InsetCommand(buf, p), include_label(uniqueID()),
preview_(make_unique(this)),
failedtoload_(false),
- set_label_(false), label_(0), child_buffer_(0)
+ set_label_(false), label_(0), child_buffer_(0), file_exist_(false)
{
preview_->connect([=](){ fileChanged(); });
@@ -203,7 +204,7 @@ InsetInclude::InsetInclude(Buffer * buf, InsetCommandParams
const & p)
InsetInclude::InsetInclude(InsetInclude const & other)
: InsetCommand(other), include_label(other.include_label),
preview_(make_unique(this)),
failedtoload_(false),
- set_label_(false), label_(0), child_buffer_(0)
+ set_label_(false), label_(0), child_buffer_(0),
file_exist_(other.file_exist_)
{
preview_->connect([=](){ fileChanged(); });
@@ -391,6 +392,8 @@ bool InsetInclude::isChildIncluded() const
docstring InsetInclude::screenLabel() const
{
+ docstring pre = file_exist_ ? docstring() : _("FILE MISSING:");
+
docstring temp;
switch (type(params())) {
@@ -418,13 +421,9 @@ docstring InsetInclude::screenLabel() const
}
temp += ": ";
+ temp += from_utf8(onlyFileName(to_utf8(params()["filename"])));
- if (params()["filename"].empty())
- temp += "???";
- else
- temp += from_utf8(onlyFileName(to_utf8(params()["filename"])));
-
- return temp;
+ return pre.empty() ? temp : pre + from_ascii(" ") + temp;
}
@@ -513,8 +512,8 @@ void InsetInclude::latex(otexstream & os, OutputParams
const & runparams) const
{
string incfile = to_utf8(params()["filename"]);
- // Do nothing if no file name has been specified
- if (incfile.empty())
+ // Do nothing if file doesn't exist
+ if (!listingFileExist())
return;
FileName const included_file = includedFileName(buffer(), params());
@@ -1330,6 +1329,8 @@ void InsetInclude::updateCommand()
void InsetInclude::updateBuffer(ParIterator const & it, UpdateType utype)
{
+ file_exist_ = listingFileExist();
+
button_.update(screenLabel(), true, false);
Buffer const * const childbuffer = getChildBuffer();
@@ -1359,4 +1360,12 @@ void InsetInclude::updateBuffer(ParIterator const & it,
UpdateType utype)
}
+bool InsetInclude::listingFileExist() const
+{
+ // check whether the file of the listing exist
+ string listingFileName = to_utf8(params()["filename"]);
+ FileName fn = support::makeAbsPath(listingFileName,
support::onlyPath(buffer().absFileName()));
+ return fn.exists();
+}
+
} // namespace lyx
diff --git a/src/insets/InsetInclude.h b/src/insets/InsetInclude.h
old mode 100644
new mode 100755
index 9ecaf42e7e..65e2945c92
--- a/src/insets/InsetInclude.h
+++ b/src/insets/InsetInclude.h
@@ -137,6 +137,8 @@ private:
void editIncluded(std::string const & file);
///
bool isChildIncluded() const;
+ /// check whether the file of the listing exist
+ bool listingFileExist() const;
/// \name Private functions inherited from Inset class
//@{
@@ -170,6 +172,7 @@ private:
mutable docstring listings_label_;
InsetLabel * label_;
mutable Buffer * child_buffer_;
+ mutable bool file_exist_;
};
Re: Bug #11484
Am Sonntag, den 10.03.2019, 19:08 +0100 schrieb Jürgen Womser-Schütz: > Ahh, I see: _() is the C++ user defined flag for enabling the > translation machinery! I couldn't find the definition :-( src/support/getttext.h Jürgen signature.asc Description: This is a digitally signed message part
Re: Bug #11484
Am 10.03.2019 um 18:26 schrieb Jürgen Spitzmüller:
> Am Sonntag, den 10.03.2019, 17:38 +0100 schrieb Jürgen Womser-Schütz:
>> PS: If the small change is worth to merge in LyX: what is about the
>> translation of the new introduced string into all supported languages
> Our l7n machinery will catch this string and add it to the po files for
> translation. The translators will handle it.
>
> BTW I would do
>
> docstring pre = file_exist_ ? docstring() : _("FILE MISSING: ");
>
> (not flag the empty string as translatable) and maybe also handle the
> trailing space outside the translatable string, since this is prone to
> be missed by translators. I.e.,
>
> docstring pre = file_exist_ ? docstring() : _("FILE MISSING:");
>
> ...
>
> return pre.empty() ? temp : pre + from_ascii(" ") + temp;
>
>> and whats about the LyX documentation (also all languages)?
Ahh, I see: _() is the C++ user defined flag for enabling the
translation machinery! I couldn't find the definition :-(
> Is there something to document here?
I am not sure. Maybe not. I will have a look.
Jürgen
Re: Bug #11484
Am Sonntag, den 10.03.2019, 17:38 +0100 schrieb Jürgen Womser-Schütz:
> PS: If the small change is worth to merge in LyX: what is about the
> translation of the new introduced string into all supported languages
Our l7n machinery will catch this string and add it to the po files for
translation. The translators will handle it.
BTW I would do
docstring pre = file_exist_ ? docstring() : _("FILE MISSING: ");
(not flag the empty string as translatable) and maybe also handle the
trailing space outside the translatable string, since this is prone to
be missed by translators. I.e.,
docstring pre = file_exist_ ? docstring() : _("FILE MISSING:");
...
return pre.empty() ? temp : pre + from_ascii(" ") + temp;
> and whats about the LyX documentation (also all languages)?
Is there something to document here?
Jürgen
signature.asc
Description: This is a digitally signed message part
Re: Bug #11484
Hi,
I have a revised patch for the little problem of an possibly non
existing user-provided listing file in LyX child-document dialog.
Following JMarc's suggestion I am now altering the label-text of the
Include-inset by an additional string "FILE MISSING: " (of course only
if the file is missing!).
So the user sees immediately what is going on instead of getting later
an LaTex-error during conversation (for example in PDF).
Hth
Jürgen
PS: If the small change is worth to merge in LyX: what is about the
translation of the new introduced string into all supported languages
and whats about the LyX documentation (also all languages)?
Am 25.02.2019 um 18:33 schrieb Jean-Marc Lasgouttes:
> Le 22/02/2019 à 21:29, Jürgen Womser-Schütz a écrit :
>> Hi alltogether,
>>
>> I have a revised patch for the little problem of possibly non existing
>> user-provided listing file in LyX child-document dialog.
>> If the listing file don't exist, a warning QMessageBox is displayed:
>> "The listing file " FILENAME " does not exist. You will not be able
>> to export your LyX-file (LaTeX-errors)."
>> The diff-file is attached.
>
> Another possibility would be to change the label of the include inset
> to start with "INVALID" of something like that. This is already what
> we do with broken cross references. This would also appear in the
> outline and the user would see it immediately after clicking OK.
>
>> In my opinion the whole LyX child-document dialog needs a revision.
>> Im am not a user of this dialog at all but if I select instead of
>> "Program Listing" for example "Input" or "Include" I get errors.
>
> This is my problem too. I am a very light user of the dialog.
>
> JMarc
>
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
old mode 100644
new mode 100755
index 51cc147333..a74a51600b
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -55,6 +55,7 @@
#include "support/convert.h"
#include "support/debug.h"
#include "support/docstream.h"
+#include "support/FileName.h"
#include "support/FileNameList.h"
#include "support/filetools.h"
#include "support/gettext.h"
@@ -188,7 +189,7 @@ char_type replaceCommaInBraces(docstring & params)
InsetInclude::InsetInclude(Buffer * buf, InsetCommandParams const & p)
: InsetCommand(buf, p), include_label(uniqueID()),
preview_(make_unique(this)),
failedtoload_(false),
- set_label_(false), label_(0), child_buffer_(0)
+ set_label_(false), label_(0), child_buffer_(0), file_exist_(false)
{
preview_->connect([=](){ fileChanged(); });
@@ -203,7 +204,7 @@ InsetInclude::InsetInclude(Buffer * buf, InsetCommandParams
const & p)
InsetInclude::InsetInclude(InsetInclude const & other)
: InsetCommand(other), include_label(other.include_label),
preview_(make_unique(this)),
failedtoload_(false),
- set_label_(false), label_(0), child_buffer_(0)
+ set_label_(false), label_(0), child_buffer_(0),
file_exist_(other.file_exist_)
{
preview_->connect([=](){ fileChanged(); });
@@ -391,6 +392,8 @@ bool InsetInclude::isChildIncluded() const
docstring InsetInclude::screenLabel() const
{
+ docstring pre = file_exist_ ? _("") : _("FILE MISSING: ");
+
docstring temp;
switch (type(params())) {
@@ -418,13 +421,9 @@ docstring InsetInclude::screenLabel() const
}
temp += ": ";
+ temp += from_utf8(onlyFileName(to_utf8(params()["filename"])));
- if (params()["filename"].empty())
- temp += "???";
- else
- temp += from_utf8(onlyFileName(to_utf8(params()["filename"])));
-
- return temp;
+ return pre + temp;
}
@@ -513,8 +512,8 @@ void InsetInclude::latex(otexstream & os, OutputParams
const & runparams) const
{
string incfile = to_utf8(params()["filename"]);
- // Do nothing if no file name has been specified
- if (incfile.empty())
+ // Do nothing if file doesn't exist
+ if (!listingFileExist())
return;
FileName const included_file = includedFileName(buffer(), params());
@@ -1330,6 +1329,8 @@ void InsetInclude::updateCommand()
void InsetInclude::updateBuffer(ParIterator const & it, UpdateType utype)
{
+ file_exist_ = listingFileExist();
+
button_.update(screenLabel(), true, false);
Buffer const * const childbuffer = getChildBuffer();
@@ -1359,4 +1360,12 @@ void InsetInclude::updateBuffer(ParIterator const & it,
UpdateType utype)
}
+bool InsetInclude::listingFileExist() const
+{
+ // check whether the file of the listing exist
+ string listingFileName = to_utf8(params()["filename"]);
+ FileName fn = support::makeAbsPath(listingFileName,
support::onlyPath(buffer().absFileName()));
+ return fn.exists();
+}
+
} // namespace lyx
diff --git a/src/insets/InsetInclude.h b/src/insets/InsetInclude.h
old mode 100644
new mode 100755
index 9ecaf42e7e..65e2945c92
---
Re: Bug #11484
Am 25.02.2019 um 18:33 schrieb Jean-Marc Lasgouttes: > Le 22/02/2019 à 21:29, Jürgen Womser-Schütz a écrit : >> Hi alltogether, >> >> I have a revised patch for the little problem of possibly non existing >> user-provided listing file in LyX child-document dialog. >> If the listing file don't exist, a warning QMessageBox is displayed: >> "The listing file " FILENAME " does not exist. You will not be able >> to export your LyX-file (LaTeX-errors)." >> The diff-file is attached. > > Another possibility would be to change the label of the include inset > to start with "INVALID" of something like that. This is already what > we do with broken cross references. This would also appear in the > outline and the user would see it immediately after clicking OK. > >> In my opinion the whole LyX child-document dialog needs a revision. >> Im am not a user of this dialog at all but if I select instead of >> "Program Listing" for example "Input" or "Include" I get errors. > > This is my problem too. I am a very light user of the dialog. > > JMarc > OK, "I will do my very best". Jürgen
Re: Bug #11484
Le 22/02/2019 à 21:29, Jürgen Womser-Schütz a écrit : Hi alltogether, I have a revised patch for the little problem of possibly non existing user-provided listing file in LyX child-document dialog. If the listing file don't exist, a warning QMessageBox is displayed: "The listing file " FILENAME " does not exist. You will not be able to export your LyX-file (LaTeX-errors)." The diff-file is attached. Another possibility would be to change the label of the include inset to start with "INVALID" of something like that. This is already what we do with broken cross references. This would also appear in the outline and the user would see it immediately after clicking OK. In my opinion the whole LyX child-document dialog needs a revision. Im am not a user of this dialog at all but if I select instead of "Program Listing" for example "Input" or "Include" I get errors. This is my problem too. I am a very light user of the dialog. JMarc
Re: Bug #11484
Hi alltogether,
I have a revised patch for the little problem of possibly non existing
user-provided listing file in LyX child-document dialog.
If the listing file don't exist, a warning QMessageBox is displayed:
"The listing file " FILENAME " does not exist. You will not be able
to export your LyX-file (LaTeX-errors)."
The diff-file is attached.
In my opinion the whole LyX child-document dialog needs a revision.
Im am not a user of this dialog at all but if I select instead of
"Program Listing" for example "Input" or "Include" I get errors.
HTH Jürgen
Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz:
> Hi,
>
> in the surrounding field of bug #11484 I tried to improve the "user
> experiance" by checking the existence of the user-specified listing file.
> I changed two files:
> src/frontends/qt4/GuiInclude.cpp
> src/frontends/qt4/GuiInclude.h
> The diff-output is attached.
>
> Jürgen
From 6beaa885b113233d741a0f4516515db7f15f4087 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCrgen=20Womser-Sch=C3=BCtz?=
Date: Fri, 22 Feb 2019 20:21:12 +0100
Subject: [PATCH] bug11484
---
src/frontends/qt4/GuiDialog.h| 2 +-
src/frontends/qt4/GuiInclude.cpp | 51
src/frontends/qt4/GuiInclude.h | 4 +++
3 files changed, 50 insertions(+), 7 deletions(-)
diff --git a/src/frontends/qt4/GuiDialog.h b/src/frontends/qt4/GuiDialog.h
index d5f4227c08..2ca0a93448 100644
--- a/src/frontends/qt4/GuiDialog.h
+++ b/src/frontends/qt4/GuiDialog.h
@@ -52,7 +52,7 @@ public Q_SLOTS:
// Restore Defaults button clicked
virtual void slotRestoreDefaults() {}
// OK button clicked
- void slotOK();
+ virtual void slotOK();
// Apply button clicked
void slotApply();
// AutoApply checkbox clicked
diff --git a/src/frontends/qt4/GuiInclude.cpp b/src/frontends/qt4/GuiInclude.cpp
index c55d71fa7f..e9b16487e6 100644
--- a/src/frontends/qt4/GuiInclude.cpp
+++ b/src/frontends/qt4/GuiInclude.cpp
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
@@ -75,11 +76,17 @@ GuiInclude::GuiInclude(GuiView & lv)
bc().setPolicy(ButtonPolicy::OkApplyCancelReadOnlyPolicy);
bc().setOK(buttonBox->button(QDialogButtonBox::Ok));
bc().setCancel(buttonBox->button(QDialogButtonBox::Cancel));
- bc().addReadOnly(filenameED);
- bc().addReadOnly(browsePB);
- bc().addReadOnly(visiblespaceCB);
- bc().addReadOnly(typeCO);
- bc().addReadOnly(listingsED);
+
+ if(false)
+ {
+ // unnecessary calls (list ButtonController.readOnly_ is used
only
+ // in ButtonController.refreshReadOnly(), but this is not
called anymore):
+ bc().addReadOnly(filenameED);
+ bc().addReadOnly(browsePB);
+ bc().addReadOnly(visiblespaceCB);
+ bc().addReadOnly(typeCO);
+ bc().addReadOnly(listingsED);
+ }
bc().addCheckedLineEdit(filenameED, filenameLA);
}
@@ -288,6 +295,23 @@ void GuiInclude::browse()
}
+void GuiInclude::slotOK()
+{
+ // listing file exists?
+ QString docpath;
+ if( !listingFileExist( docpath ) )
+ {
+ // warning: file doesn't exist
+ QMessageBox::information(this, qt_("LyX Warning"),
+qt_("The listing file \"") + docpath + qt_("\" does not exist.
"
+"You will not be able to export your LyX-file
(LaTeX-errors)."));
+ }
+
+ // call base-class functionality
+ GuiDialog::slotOK();
+}
+
+
void GuiInclude::edit()
{
if (!isValid())
@@ -303,10 +327,25 @@ void GuiInclude::edit()
bool GuiInclude::isValid()
{
- return !filenameED->text().isEmpty() &&
validate_listings_params().empty();
+ return validate_listings_params().empty();
}
+bool GuiInclude::listingFileExist(QString & docpath)
+{
+ // check whether the file of the listing exists
+ QString const listingFileName = filenameED->text();
+ docpath = toqstr(support::onlyPath(buffer().absFileName()));
+ docpath += listingFileName;
+
+ bool listingFileExist = false;
+ if (!listingFileName.isEmpty())
+ listingFileExist = FileName(docpath.toStdString()).exists();
+
+ return listingFileExist;
+ }
+
+
QString GuiInclude::browse(QString const & in_name, Type in_type) const
{
QString const title = qt_("Select document to include");
diff --git a/src/frontends/qt4/GuiInclude.h b/src/frontends/qt4/GuiInclude.h
index 1106c3e02e..9da767352a 100644
--- a/src/frontends/qt4/GuiInclude.h
+++ b/src/frontends/qt4/GuiInclude.h
@@ -72,6 +72,8 @@ private:
void updateLists();
/// validate listings parameters and return an error message, if any
docstring validate_listings_params();
+ /// exists the file?
+ bool listingFileExist(QString &);
///
bool isValid();
/// Apply changes
@@ -80,6 +82,8 @@
Re: Bug #11484
Hi alltogether, I hereby grant permission to use my work for LyX under the license GPL version 2 or later. Best regards Jürgen Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz: > Hi, > > in the surrounding field of bug #11484 I tried to improve the "user > experiance" by checking the existence of the user-specified listing file. > I changed two files: > src/frontends/qt4/GuiInclude.cpp > src/frontends/qt4/GuiInclude.h > The diff-output is attached. > > Jürgen
Re: Bug #11484
Am 10 Feb 2019 um 09:23:53 schrieb Jean-Marc Lasgouttes: > Dear Jürgen, > > I have a question: is the effect of your patch to disable the OK button when > the file does not exist? If it is the case, I am not sure that it will > considered as an improvement by everyone. I am not a heavy listings user, but > I suspect that some people insert references to documents with placeholder > names that they will update/create later. > > > I think it would be better to warn in a different way. > > What I am trying to do is just start the discussion. It is up to heavy users > of this feature to tell how they use it. > > > JMarc > > PS: welcome anyway, and sorry for the delay in answering :) Hello JMarc, you are right: to get the most minimal change, I intended the same behaviour as in the original version if no filename whatsoever is provided by the user. If you have an result for your discussion: please tell it to me. In the meantime I try to find out the usual LyX-way of handling such a warning. Best regards Jürgen PS: Because of an unknown problem I didn't receive your mail. I found it "accidently" visiting the LyX-mail-archive :-(
Re: Bug #11484
Le 06/02/2019 à 16:25, Jürgen Womser-Schütz a écrit : Hi, in the surrounding field of bug #11484 I tried to improve the "user experiance" by checking the existence of the user-specified listing file. I changed two files: src/frontends/qt4/GuiInclude.cpp src/frontends/qt4/GuiInclude.h The diff-output is attached. Dear Jürgen, I have a question: is the effect of your patch to disable the OK button when the file does not exist? If it is the case, I am not sure that it will considered as an improvement by everyone. I am not a heavy listings user, but I suspect that some people insert references to documents with placeholder names that they will update/create later. I think it would be better to warn in a different way. What I am trying to do is just start the discussion. It is up to heavy users of this feature to tell how they use it. JMarc PS: welcome anyway, and sorry for the delay in answering :)
Re: Bug #11484
Am Mittwoch, 6. Februar 2019 17:57:55 CET schrieb Jürgen Womser-Schütz : > ... and a patch to git-master for convenience. > > Jürgen > Jürgen, if we would apply your patches, we would need your explicit permission. Like a short message on this list containing: "I hereby grant permission to use my work for LyX under the license GPL version 2 or later." Kornel signature.asc Description: This is a digitally signed message part.
Re: Bug #11484
Hi, you are right - maybe I am a bit "over-enthusiastic" :-) Jürgen Am 09.02.2019 um 22:48 schrieb Richard Kimberly Heck: > On 2/8/19 8:53 PM, Scott Kostyshak wrote: >> On Fri, Feb 08, 2019 at 10:40:48PM +0100, Kornel Benko wrote: >>> Am Freitag, 8. Februar 2019 22:14:55 CET schrieb Jürgen Womser-Schütz >>> : Hi alltogether, as I can see: no volunteers desired. Best regards Jürgen Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz: > Hi, > > in the surrounding field of bug #11484 I tried to improve the "user > experiance" by checking the existence of the user-specified listing file. > I changed two files: > src/frontends/qt4/GuiInclude.cpp > src/frontends/qt4/GuiInclude.h > The diff-output is attached. > > Jürgen >>> Don't be disappointed. Sometimes it may take some time until some >>> experienced >>> developer checks your patch. >> +1 > And, just to be clear, traffic on the list has just generally been a bit > light lately. My sense is that a lot of people are very busy right now. > We're *all* volunteers with demanding day-jobs. I am as busy with > classes, etc, right now as I can ever remember being. And now I'm sick. :-( > > Riki > >
Re: Bug #11484
On 2/8/19 8:53 PM, Scott Kostyshak wrote: > On Fri, Feb 08, 2019 at 10:40:48PM +0100, Kornel Benko wrote: >> Am Freitag, 8. Februar 2019 22:14:55 CET schrieb Jürgen Womser-Schütz >> : >>> Hi alltogether, as I can see: no volunteers desired. >>> Best regards Jürgen >>> >>> >>> Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz: Hi, in the surrounding field of bug #11484 I tried to improve the "user experiance" by checking the existence of the user-specified listing file. I changed two files: src/frontends/qt4/GuiInclude.cpp src/frontends/qt4/GuiInclude.h The diff-output is attached. Jürgen >> Don't be disappointed. Sometimes it may take some time until some experienced >> developer checks your patch. > +1 And, just to be clear, traffic on the list has just generally been a bit light lately. My sense is that a lot of people are very busy right now. We're *all* volunteers with demanding day-jobs. I am as busy with classes, etc, right now as I can ever remember being. And now I'm sick. :-( Riki
Re: Bug #11484
On Fri, Feb 08, 2019 at 10:40:48PM +0100, Kornel Benko wrote: > Am Freitag, 8. Februar 2019 22:14:55 CET schrieb Jürgen Womser-Schütz > : > > Hi alltogether, as I can see: no volunteers desired. > > Best regards Jürgen > > > > > > Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz: > > > Hi, > > > > > > in the surrounding field of bug #11484 I tried to improve the "user > > > experiance" by checking the existence of the user-specified listing file. > > > I changed two files: > > > src/frontends/qt4/GuiInclude.cpp > > > src/frontends/qt4/GuiInclude.h > > > The diff-output is attached. > > > > > > Jürgen > > > > Don't be disappointed. Sometimes it may take some time until some experienced > developer checks your patch. +1 > It is surely not forgotten. True, after a couple of days, it should not be viewed as forgotten. However, if no response is given in a week or two, you're welcome to politely bump your email. I usually do something like bump my email after a week, and then if no one still responds, bump after another month saying "OK this is the last time I'm bumping; otherwise I'll assume there is no interest." Scott signature.asc Description: PGP signature
Re: Bug #11484
Am Freitag, 8. Februar 2019 22:14:55 CET schrieb Jürgen Womser-Schütz : > Hi alltogether, as I can see: no volunteers desired. > Best regards Jürgen > > > Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz: > > Hi, > > > > in the surrounding field of bug #11484 I tried to improve the "user > > experiance" by checking the existence of the user-specified listing file. > > I changed two files: > > src/frontends/qt4/GuiInclude.cpp > > src/frontends/qt4/GuiInclude.h > > The diff-output is attached. > > > > Jürgen > Don't be disappointed. Sometimes it may take some time until some experienced developer checks your patch. It is surely not forgotten. Kornel signature.asc Description: This is a digitally signed message part.
Re: Bug #11484
Hi alltogether, as I can see: no volunteers desired. Best regards Jürgen Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz: > Hi, > > in the surrounding field of bug #11484 I tried to improve the "user > experiance" by checking the existence of the user-specified listing file. > I changed two files: > src/frontends/qt4/GuiInclude.cpp > src/frontends/qt4/GuiInclude.h > The diff-output is attached. > > Jürgen
Re: Bug #11484
... and a patch to git-master for convenience.
Jürgen
Am 06.02.2019 um 16:25 schrieb Jürgen Womser-Schütz:
> Hi,
>
> in the surrounding field of bug #11484 I tried to improve the "user
> experiance" by checking the existence of the user-specified listing file.
> I changed two files:
> src/frontends/qt4/GuiInclude.cpp
> src/frontends/qt4/GuiInclude.h
> The diff-output is attached.
>
> Jürgen
From 559a25f02c3f1d35e31b7cd1adcbf5609175c296 Mon Sep 17 00:00:00 2001
From: JWS
Date: Wed, 6 Feb 2019 17:39:35 +0100
Subject: [PATCH] Improve the "user experience" by checking the existence
---
src/frontends/qt4/GuiInclude.cpp | 17 -
src/frontends/qt4/GuiInclude.h | 2 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/frontends/qt4/GuiInclude.cpp b/src/frontends/qt4/GuiInclude.cpp
index c55d71fa7f..4d96ac8415 100644
--- a/src/frontends/qt4/GuiInclude.cpp
+++ b/src/frontends/qt4/GuiInclude.cpp
@@ -303,7 +303,22 @@ void GuiInclude::edit()
bool GuiInclude::isValid()
{
- return !filenameED->text().isEmpty() &&
validate_listings_params().empty();
+ return listingFileExist() && validate_listings_params().empty();
+}
+
+
+bool GuiInclude::listingFileExist()
+{
+ // check whether the file of the listing exists
+ QString const listingFileName = filenameED->text();
+ QString docpath = toqstr(support::onlyPath(buffer().absFileName()));
+ docpath += listingFileName;
+
+ bool listingFileExist = false;
+ if (!listingFileName.isEmpty())
+ listingFileExist = FileName(docpath.toStdString()).exists();
+
+ return listingFileExist;
}
diff --git a/src/frontends/qt4/GuiInclude.h b/src/frontends/qt4/GuiInclude.h
index 1106c3e02e..862cdab968 100644
--- a/src/frontends/qt4/GuiInclude.h
+++ b/src/frontends/qt4/GuiInclude.h
@@ -72,6 +72,8 @@ private:
void updateLists();
/// validate listings parameters and return an error message, if any
docstring validate_listings_params();
+ /// exists the file?
+ bool listingFileExist();
///
bool isValid();
/// Apply changes
--
2.20.1
