Re: misleading error message from variable modifier
> If you'd like to augment the test suite where you feel it lacks something, > please feel free to do so. tests were included in my patch. you deleted them. i think they should be added in. > are there any tests that cover the variable modifiers, either unit tests > or > > functional tests? > Yes. The test framework and tests are available for you to see. my question was whether you have tests for the variable modifiers. i don't see any. that's the area of code i was touching, and that's why i wrote a few tests of that area. On Thu, Mar 8, 2018 at 7:10 AM, Chet Ramey wrote: > On 3/7/18 1:00 PM, don fong wrote: > > Chet Ramey wrote: > >> What are the most important features that you consider to lack > unit tests? > > > > are there any tests that cover the variable modifiers, either unit tests > or > > functional tests? > > Yes. The test framework and tests are available for you to see. They're all > tests of shell behavior, since what concerns most people is shell behavior. > If you'd like to augment the test suite where you feel it lacks something, > please feel free to do so. I'm not interested in adding a new testing > framework. > > -- > ``The lyf so short, the craft so long to lerne.'' - Chaucer > ``Ars longa, vita brevis'' - Hippocrates > Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/ >
Re: indirect variable behavior breakage
On 3/7/18 3:20 PM, christopher barry wrote: > On Wed, 7 Mar 2018 11:45:13 -0500 > christopher barry wrote: > > ===8<---snip > >> >> I am in fact using this method with associative arrays. >> >> I have a default hash that is full of default values for a particular >> generic type of thing. Variations of this thing use many of the >> defaults, but also set some values uniquely. I could simply duplicate >> this hash everywhere, and edit the differences, but then changing >> any defaults would require a change everywhere. >> >> As an alternative, I created a template hash that all non-default >> things start out with. >> >> this template hash has all of it's values pointing to >> "default_hash[somekey]" >> >> new things that need to change their specific values simply override >> that and put in their string. >> >> Now, changes to actual values in default_hash are seen everywhere they >> are not overridden locally by the specific thing using it. >> >> so reading a hash value may be an indirect pointer to the default_hash >> or an overridden value set by the specific thing using the hash. >> >> ivar solves this nicely. >> >> >> But to get back on point, the bug in bash was nasty, and it's >> fantastic that it's fixed now, but throwing an error now, rather than >> simply returning 1 with no data is a bit like throwing the baby out >> with the bathwater. >> >> >> -C > > Chet, > > Does modifying this current behavior sound like anything you would be > willing to entertain? I think that throwing an error when the value of a variable used for indirection would not be accepted as valid (and throw an error) when used directly is the right thing to do, and I will leave it as the default behavior. I'm open to suggestions about how to allow alternate behavior: a shell option, something controlled by the shell compatibility level, something else? It seems like you're really interested in learning whether a particular string is a valid shell variable and doing something based on the result. You could check that directly -- the pattern to describe a shell variable isn't that hard to write -- but I get the sense that you'd rather not take the extra step. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: indirect variable behavior breakage
On Fri, 9 Mar 2018 11:29:35 -0500
Chet Ramey wrote:
> On 3/7/18 3:20 PM, christopher barry wrote:
> > On Wed, 7 Mar 2018 11:45:13 -0500
> > christopher barry wrote:
> >
> > ===8<---snip
> >
> >>
> >> I am in fact using this method with associative arrays.
> >>
> >> I have a default hash that is full of default values for a
> >> particular generic type of thing. Variations of this thing use
> >> many of the defaults, but also set some values uniquely. I could
> >> simply duplicate this hash everywhere, and edit the differences,
> >> but then changing any defaults would require a change everywhere.
> >>
> >> As an alternative, I created a template hash that all non-default
> >> things start out with.
> >>
> >> this template hash has all of it's values pointing to
> >> "default_hash[somekey]"
> >>
> >> new things that need to change their specific values simply
> >> override that and put in their string.
> >>
> >> Now, changes to actual values in default_hash are seen everywhere
> >> they are not overridden locally by the specific thing using it.
> >>
> >> so reading a hash value may be an indirect pointer to the
> >> default_hash or an overridden value set by the specific thing
> >> using the hash.
> >>
> >> ivar solves this nicely.
> >>
> >>
> >> But to get back on point, the bug in bash was nasty, and it's
> >> fantastic that it's fixed now, but throwing an error now, rather
> >> than simply returning 1 with no data is a bit like throwing the
> >> baby out with the bathwater.
> >>
> >>
> >> -C
> >
> > Chet,
> >
> > Does modifying this current behavior sound like anything you would
> > be willing to entertain?
>
> I think that throwing an error when the value of a variable used for
> indirection would not be accepted as valid (and throw an error) when
> used directly is the right thing to do, and I will leave it as the
> default behavior.
>
> I'm open to suggestions about how to allow alternate behavior: a shell
> option, something controlled by the shell compatibility level,
> something else? It seems like you're really interested in learning
> whether a particular string is a valid shell variable and doing
> something based on the result. You could check that directly -- the
> pattern to describe a shell variable isn't that hard to write -- but
> I get the sense that you'd rather not take the extra step.
>
> Chet
>
Hi Chet,
It's not that I don't want to take any required steps, it was that it
seemed logical to me to simply return 1 in that instance. When I
discovered that this method worked the way I was hoping, I was kinda
miffed at myself for never having noticed it earlier. Then I was shown
it was not working on new versions. I had never hit the core dump issue
before you pointed it out.
Could the fact that that bug caused a core dump be influencing why you
do not want to simply return 1 now? Maybe throwing an error now seems
better because it was such a bad bug before?
When assigning to a var, format checking absolutely should be done, and
an error should be thrown if invalid. But if only checking, e.g. just
reading a name, then returning 1 if it's an invalid name or does not
exist seems like the smallest, most applicable hammer for the job at
hand.
I did actually spend some time (longer than I would like to admit)
trying to craft a decent regex to make sure that every possible way of
asking was covered, but it turns out to be a bit more complex that one
may think at first blush.
^([#]*(_[[:digit:]]|[_[:alpha:]])[_[:alnum:]]*)(\[[_[:alnum:]*@]+\])$
testing against ${1}, ${BASH_REMATCH[0]} and ${BASH_REMATCH[1]} would
mostly show the valid forms, but this regex was still not quite right.
A colleague here at work came up with a beautiful 16oz ball-peen hammer
approach, no regex required, as can be seen here:
#---
function ivar()
# Description: cascading indirect -> regular var expander
{
( echo -n "${!1:-${1}}" ) 2>/dev/null || echo -n "${1}"
}
#---
it solves the problem, so that's what I'm using now, but it's obviously
not as clean and simple as it was, and it can still core dump the
sub-shell, which does suck, but it's non-fatal.
If you can't see my point about not needing to throw an error on just
a read, then I guess I'll just have to deal with it.
Thanks, and maybe you will come to see my position at some point, never
know :)
Regards,
-C
Re: Unexpected behavior of 'declare +n var' when var's target is unset and undeclared
On 3/8/18 2:51 PM, GreatBigDot wrote: > Bash Version: 4.4 > Patch Level: 19 > Release Status: release > > Description: > If you try to turn a nameref back a regular variable with > 'declare -n' but the nameref is pointing to an undeclared variable, the > nameref remains a nameref, and the variable it points to is declared > (but remians unset). Thanks for the report. This is a bug: the code neglected to remove the nameref attribute from `foo' before tying to modify the attributes of the referenced variable (`bar'). It will be fixed in the next devel snapshot. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
[PATCH] Add active mark, face support; activate mark on paste
This patch teaches readline about two concepts from Emacs: 1) faces,
and 2) the mark being "active". Both exist in rudimentary form: we
support exactly two faces, normal and "standout", and use standout to
highlight the contents of the region when the mark is active. Readline
redisplay is now smart enough to consider not only text content, but
also face differences when computing how to refresh the screen.
The immediate motivation for this patch is to provide visual feedback
to users after a bracketed paste operation, but an active region
concept could be useful for all kinds of things, including shift-arrow
selection or integration with xterm mouse facilities.
The mark automatically deactivates after most commands, including C-g
and cursor motion. We do keep any active mark active across screen
refresh operations, however.
exchange-point-and-mark activates the mark just like in Emacs.
---
lib/readline/display.c | 478 +++
lib/readline/kill.c | 6 +-
lib/readline/readline.c | 19 ++
lib/readline/readline.h | 4 +
lib/readline/rlprivate.h | 5 +-
lib/readline/terminal.c | 43
lib/readline/text.c | 55 +++--
lib/readline/util.c | 1 +
8 files changed, 398 insertions(+), 213 deletions(-)
diff --git a/lib/readline/display.c b/lib/readline/display.c
index 2d2e768a..f228a39e 100644
--- a/lib/readline/display.c
+++ b/lib/readline/display.c
@@ -63,13 +63,13 @@
extern char *strchr (), *strrchr ();
#endif /* !strchr && !__STDC__ */
-static void update_line PARAMS((char *, char *, int, int, int, int));
+static void update_line PARAMS((char *, char *, char *, char *, int, int, int,
int));
static void space_to_eol PARAMS((int));
static void delete_chars PARAMS((int));
-static void insert_some_chars PARAMS((char *, int, int));
static void open_some_spaces PARAMS((int));
static void cr PARAMS((void));
static void redraw_prompt PARAMS((char *));
+static void _rl_move_cursor_relative PARAMS((int, const char *, const char *));
/* Values for FLAGS */
#define PMT_MULTILINE 0x01
@@ -80,6 +80,7 @@ static char *expand_prompt PARAMS((char *, int, int *, int *,
int *, int *));
struct line_state
{
char *line;
+char *line_face;
int *lbreaks;
int lbsize;
#if defined (HANDLE_MULTIBYTE)
@@ -102,7 +103,9 @@ static int line_structures_initialized = 0;
#define vis_lbsize (line_state_visible->lbsize)
#define visible_line (line_state_visible->line)
+#define visible_face (line_state_visible->line_face)
#define invisible_line (line_state_invisible->line)
+#define invisible_face (line_state_invisible->line_face)
#if defined (HANDLE_MULTIBYTE)
static int _rl_col_width PARAMS((const char *, int, int, int));
@@ -123,7 +126,10 @@ static int _rl_col_width PARAMS((const char *, int, int,
int));
to use prompt_last_invisible directly. */
#define PROMPT_ENDING_INDEX \
((MB_CUR_MAX > 1 && rl_byte_oriented == 0) ? prompt_physical_chars :
prompt_last_invisible+1)
-
+
+#define FACE_NORMAL '0'
+#define FACE_STANDOUT '1'
+#define FACE_INVALID ((char)1)
/* */
/* */
@@ -208,8 +214,8 @@ static int msg_bufsiz = 0;
/* Non-zero forces the redisplay even if we thought it was unnecessary. */
static int forced_display;
-/* Default and initial buffer size. Can grow. */
-static int line_size = 1024;
+/* Line buffer size. */
+static int line_size;
/* Variables to keep track of the expanded prompt string, which may
include invisible characters. */
@@ -521,6 +527,64 @@ rl_expand_prompt (prompt)
}
}
+static void
+ensure_line_size (minsize)
+ int minsize;
+{
+ int minimum_size = 1024;
+ int new_line_size, delta;
+ if (minsize < minimum_size)
+minsize = minimum_size;
+ if (line_size >= minsize)
+return;
+ if (!new_line_size)
+new_line_size = minimum_size;
+ while (new_line_size < minsize)
+new_line_size *= 2;
+ visible_line = (char *)xrealloc (visible_line, new_line_size);
+ visible_face = (char *)xrealloc (visible_face, new_line_size);
+ invisible_line = (char *)xrealloc (invisible_line, new_line_size);
+ invisible_face = (char *)xrealloc (invisible_face, new_line_size);
+ delta = new_line_size - line_size;
+ memset (visible_line + line_size, 0, delta);
+ memset (visible_face + line_size, FACE_NORMAL, delta);
+ memset (invisible_line + line_size, 1, delta);
+ memset (invisible_face + line_size, FACE_INVALID, delta);
+ line_size = new_line_size;
+}
+
+static void
+invis_add (outp, c, face)
+ int *outp;
+ char c;
+ char face;
+{
+ ensure_line_size (*outp + 1);
+ invisible_line[*outp] = c;
+ invisible_face[*outp] = face;
+ *outp += 1;
+}
+
+static void
+invis_add_n (outp, str, n, face)
+ int *outp;
+ const char *str;
+ int n;
+ char face;
+{
+ int i;
+ for (i = 0; i < n; ++i)
+invis_add (outp, str[i], face);
+}
+
