Add new attribute inserts only heading not whole column

Bug #698575 reported by signality
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
gEDA
Fix Released
Low
Peter TB Brett

Bug Description

The version of gattrib I'm using (1.4.0.20080127 from the Debian lenny
repos) has the option to add a new attribute column (Edit > Add new
attribute column).

The problem is that this inserts a new heading but not the whole new column so what happens is that the headings after the newly inserted one shift right but the data in the columns underneath them does not. Hence the attribute values end up being renamed as different attributes.

Saving this then corrupt the whole schematic. Since there is no "undo" function, the only safe option is to cancel and lose all changes.

I'll try to illustrate what happens.

Say all the components in my schematic have attributes A, B and D and I want to add a new attribute C to all the components.

All my components have attribute information already entered for each of A, B and D.

When I run gattrib, I get a table with column headings A, B and D with values under each of the relevant columns.

If I add now a new column C then a new column heading for C is inserted between columns B and D and the existing column D heading is shifted one column to the right.

However, no new column is inserted under the C heading so the existing column contents do not shift right. Therefore, the column contents that were originally under column heading D are now under column heading C whilst the column under heading D is empty.

If I save this then of course all the component attribute values that were originally named D are now named C.

If instead, before I run gattrib, I add the new attribute to at least one instance of a component in the schematic then of course when gattrib is run the table has all four column headings A, B, C and D with the attribute values in the right columns.

This is clearly not what should happen and seems to reflect the behaviour of gattrib prior to the introduction of the add new attribute function but with the new function partly implemented.

The attached screenshots show gattrib before and after and the effect on the attributes of one component in the associated gschem schematic.

Thanks,

Andy.

Revision history for this message
signality (signality) wrote :
Revision history for this message
signality (signality) wrote :
Revision history for this message
signality (signality) wrote :
Revision history for this message
signality (signality) wrote :
Revision history for this message
signality (signality) wrote :

This should have gone in the text but I can't find a way to edit the submission (new to this app).

Obviously all the values that end up under the wrong column heading can be manually cut and pasted to the right but this is very tedious because it can only be done one cell at a time.

Also, other relevant info:
OS: Mepis 8.0.6 64 bit Linux.
PC: Athlon 64 bit dual core, 8Gb RAM.
gEDA installation from Lenny 64bit repos.

Revision history for this message
Krzysztof Kościuszkiewicz (k-kosciuszkiewicz) wrote :

Reproducible with gEDA 1.7.0.

Changed in geda:
status: New → Confirmed
importance: Undecided → Low
Joe Eli Mcilvain (jemc)
Changed in geda:
assignee: nobody → Joe Mac (joe-eli-mac)
Revision history for this message
Joe Eli Mcilvain (jemc) wrote :

I first saw bug: https://bugs.launchpad.net/geda/+bug/698608, which seems to be a duplicate of this bug, and came up with a patch that corrects the behavior and inserts the empty data column under the right column heading. The problem was that the header gets alphabetically sorted into the others (after device, footprint, and value) but the code immediately following that inserts the new column after the last column, which seems to be making the assumption that the headers do not get sorted.

My patch adds a new s_string_list function (s_string_list_find_in_list()) that searches for a given string in the list and returns the index (or -1, if it's not there), unlike the existing function (s_string_list_in_list()) (that I copied from but didn't touch) which just returns 1 as soon as it finds a matching string, or 0, if it's not there. I then added code in s_toplevel_add_new_attrib() that uses the new function to determine where the new column header was placed and insert the new data column at that index (instead of at the end).

However, doing this patch raised concerns for me about how gattrib handles "duplicate" attributes - attributes with the same name. The answer is that it doesn't handle them very well at all (it not only does not behave as expected, but can actually corrupt your schematics if you're relying on duplicate attributes). I'm not sure why you would want to have duplicate attributes, gschem seems to support them just fine, so I'm inclined to say that I should revise a lot of the gattrib code to handle them properly. I'm willing to put the time in for this, but I want input from somebody in authority on this project about what the expected behavior should be. See my bug report on this matter: https://bugs.launchpad.net/geda/+bug/980989

Peter TB Brett (peter-b)
Changed in geda:
assignee: Joe Mac (joe-eli-mac) → Peter TB Brett (peter-b)
Peter TB Brett (peter-b)
Changed in geda:
milestone: none → 1.8.0
Peter TB Brett (peter-b)
Changed in geda:
status: Confirmed → In Progress
Revision history for this message
gpleda.org commit robot (gpleda-launchpad-robot) wrote :

Bug was fixed by a commit
git stable-1.8 commit e9b3fa86d721324688ab6d28bfbc2f417e7828a0
http://git.geda-project.org/geda-gaf/commit/?id=e9b3fa86d721324688ab6d28bfbc2f417e7828a0

commit e9b3fa86d721324688ab6d28bfbc2f417e7828a0
Author: Joe Mac <email address hidden>
Commit: Peter TB Brett <email address hidden>

    gattrib: Make "Add new attribute column" actually insert column.

    I first saw bug 698608, which seems to be a duplicate of this bug, and
    came up with a patch that corrects the behavior and inserts the empty
    data column under the right column heading. The problem was that the
    header gets alphabetically sorted into the others (after device,
    footprint, and value) but the code immediately following that inserts
    the new column after the last column, which seems to be making the
    assumption that the headers do not get sorted.

    My patch adds a new s_string_list function
    (s_string_list_find_in_list()) that searches for a given string in the
    list and returns the index (or -1, if it's not there), unlike the
    existing function (s_string_list_in_list()) (that I copied from but
    didn't touch) which just returns 1 as soon as it finds a matching
    string, or 0, if it's not there. I then added code in
    s_toplevel_add_new_attrib() that uses the new function to determine
    where the new column header was placed and insert the new data column
    at that index (instead of at the end).

    Closes-bug: lp-698575
    Reviewed-by: Peter TB Brett <email address hidden>

Changed in geda:
status: In Progress → Fix Committed
Peter TB Brett (peter-b)
Changed in geda:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.