Adds three new elementInformation keys — width, height, depth (in mm)
— alongside the existing manufacturer/manufacturer_reference fields.
These describe the physical dimensions of the device a symbol
represents, set once per element definition.
Values are restricted to plain decimal numbers via a QDoubleValidator
on the information tree's item delegate (fixed-point, "." as decimal
separator via QLocale::c(), independent of the UI language), so a
later consumer can always parse them with toDouble() without
additional sanitization.
Fix bugtracker #281: new-part wizard's element editor opens behind main window
Cannot reproduce on Debian/GNU Linux, but implementation is reasonable and clean!
Fix bugtracker #251: title block template with slash in name fails silently
There are some characters that are not allowed in filenames.
Absolutely correct to mark a filename containing (one of) them as invalid!
GitHub issue #663: the "Informations" tab in the element editor's
properties dialog was only made visible for Simple and Master basetypes
(setTabVisible gate in on_m_base_type_cb_currentIndexChanged), hiding it
entirely for Slave and Terminal Block elements.
This wasn't a data-model limitation: ElementData::m_informations is read
and written identically for every basetype (elementdata.cpp), and
updateTree() already special-cased Terminal as enabled and injected
PLC-specific info rows for PLC Slave elements - that logic was simply
unreachable because the tab itself was hidden for both types. Also
flipped updateTree()'s Slave case from setDisabled to setEnabled so the
tree is actually editable once visible, matching Terminal's existing
behavior.
This lets users attach manufacturer/part-number/reference metadata
directly to Terminal Block and Slave (e.g. multi-part contactor)
elements, as requested in the issue - useful when a Slave's part number
differs from its Master's (e.g. a contactor's auxiliary contact block
vs. its coil).
Not verified: interactive element-editor GUI testing wasn't performed
in this sandbox; verified via clean incremental build only.
TitleBlockTemplate::listOfVariables() -- which scans a title block
template's cells to auto-populate the "Custom" tab in Project
Properties (a feature recently added by another contributor, see
TitleBlockPropertiesWidget::addTemplateVariables()) -- only matched
the braced "%{name}" placeholder form. The bare "%name" form (also a
legitimate, fully-supported substitution syntax -- see
TitleBlockTemplate::interpreteVariables(), which already replaces both
forms) was never matched at all, not merely mishandled on edge cases:
a cell containing "%name2" alone, "%name2 " with a trailing space, or
"%name2 %name3" with two bare variables all produced zero detected
variables, exactly matching the report (manually adding the variable
in Project Properties works fine and renders correctly, since
rendering goes through interpreteVariables()'s simple string
replacement against already-known keys, not this regex).
Fix: extend the regex to also match a bare "%name" as the longest run
of identifier characters immediately after '%', via a second
alternative/capture group. This naturally stops at whitespace, so
"%name2 " and "%name2 %name3" are both now correctly detected -- no
change to the existing braced-form handling, and the existing
globalMatch() loop already correctly finds multiple matches per cell.
Verified: clean rebuild, only the intended object file recompiled and
linked successfully. Wrote a standalone test of the regex/extraction
logic covering exactly the reported repro cases -- "%name2", "%name2 "
(trailing space), "%name2 %name3" (two bare variables), "%{name2}"
(braced form, unaffected), a braced+bare mix, plain text with no
variables, and two built-in-style names -- all extracted correctly
with no regressions to the previously-working braced form.
Not verified: the actual Project Properties "Custom" tab UI
auto-populating live, since exercising the full Xvfb GUI flow (title
block template editor > add a bare-form custom variable to a cell >
save > open Project Properties > select that template > confirm the
Custom tab lists it) was out of scope for the time available given the
extraction logic itself was already precisely verified in isolation.
DynamicElementTextItem's slave cross-reference sub-item
(m_slave_Xref_item, the small "_(1-D3)_"-style text next to a slave
element pointing back to its master) hardcoded Qt::black in three
places: on creation, on hover-leave, and when restoring color after
text editing. The PARENT text item's color is a real, user-configurable
property (color()/setColor(), persisted in the diagram XML, exposed in
the text editor's color picker) -- but the slave-Xref sub-item never
used it, so a user applying a dark theme/stylesheet had no way to make
this specific text visible even by explicitly setting a text color,
unlike every other text item in the diagram.
Fix: use color() (the parent DynamicElementTextItem's own configured
color, inherited from DiagramTextItem) instead of the Qt::black
constant in all three places. This doesn't change the default
appearance (color() defaults to black, same as before) but makes the
slave-Xref text finally respect whatever color the user sets on the
parent text field, giving dark-theme users the same escape hatch
already available for all other diagram text.
Verified: clean rebuild, only the intended object file recompiled and
linked successfully.
Not verified: a live visual confirmation of the slave-Xref text
picking up a non-default color, since reproducing this requires
constructing a master/slave-linked element pair with composite text
containing %{label} in an actual multi-folio project, which was out of
scope for the time available. Confidence rests on this being a direct,
mechanical substitution of an existing, already-used accessor
(color()) for a hardcoded constant, applied identically to the exact
three call sites that previously hardcoded Qt::black for this item,
with no other logic changed.
Saving a new user title block template (right-click "Cartouches
utilisateur" > "Nouveau modèle" > "Enregistrer sous") with a name
containing a slash (or other filesystem-reserved character) silently
did nothing, with no error shown. The entered name is turned directly
into a filename (TitleBlockTemplatesFilesCollection::toFileName()), so
e.g. "foo/bar" becomes a path "foo/bar.titleblock" -- since "foo/"
essentially never exists as a directory, the underlying file write
fails, but that failure was never surfaced:
- TitleBlockTemplateLocation::isValid() only checked for an empty
name, so an invalid name still counted as "valid" and got passed
through to save.
- QETTitleBlockTemplateEditor::saveAs(const TitleBlockTemplateLocation&)
discarded the bool result of setTemplateXmlDescription() and
unconditionally returned true, marking the undo stack clean as if
the save had actually succeeded.
Fix:
- isValid() now also rejects names containing \ / : * ? " < > |,
matching the character set that's actually unsafe once the name
becomes a filename.
- saveAs() (the no-arg entry point that asks the user for a location)
now shows a clear error dialog when the entered name is rejected,
distinguishing "user cancelled" (location.name() empty) from
"name was invalid" (non-empty but rejected by isValid()).
- saveAs(location) now checks setTemplateXmlDescription()'s return
value and shows an error dialog instead of reporting false success
on any future/other write failure, not just this one.
Verified: clean rebuild, only the intended files recompiled and
linked successfully. Live-tested under Xvfb: creating a new template
and using "Enregistrer sous" with the name "foo/bar" now shows
"Le nom « foo/bar » n'est pas valide : il ne doit pas contenir les
caractères suivants : \ / : * ? " < > |" instead of silently doing
nothing; reopening the save-as dialog afterward showed the name field
correctly empty (nothing was partially written). Saving again with a
valid name ("mytemplate_valid") completed with no error dialog, and
the resulting mytemplate_valid.titleblock file was confirmed present
on disk in the user's title-block collection directory.
ProjectView::askUserForFilePath() only appended the .qet extension when
FLATPAK_ID/SNAP_NAME were NOT set, on the assumption that the
xdg-desktop-portal file dialog used by sandboxed Snap/Flatpak builds
always appends the selected filter's extension itself (avoiding a
double ".qet.qet"). In practice, on the reporter's Snap/Ubuntu 22.04
setup the portal dialog does not append it, so the environment-based
skip left Save As producing a file with no extension at all.
Portal behavior isn't something QET controls or can reliably detect via
environment variables -- it depends on the desktop's actual portal
implementation/version. Rather than guessing per-environment, normalize
unconditionally: strip any existing .qet suffix (case-insensitive) and
re-append exactly one. This produces the correct single extension
whether or not the dialog already added it, on every environment.
Verified: clean rebuild, only the intended object file recompiled and
linked successfully. Wrote a standalone test of the normalization logic
covering no-extension, already-has-extension, uppercase-extension, and
a literal dot in the base filename -- all four produced exactly one
correct ".qet" suffix with no double-extension and no missing extension.
Not verified: the actual Snap-sandboxed portal dialog behavior itself,
since building/running the Snap package and testing its file-save
dialog under a portal is outside what's practical to set up in this
sandbox. Confidence rests on the fix removing the environment-guessing
entirely in favor of unconditional, dialog-implementation-agnostic
normalization, which is correct regardless of what the underlying
dialog does.
Double-clicking a placed element (or right-click > "Éditer l'élément")
opens its properties via Element::editProperty(), which constructs a
PropertiesEditorDialog with a real parent (QApplication::activeWindow()).
On macOS, a QDialog that has both a parent and Qt::WindowModal set falls
back to Cocoa's automatic sheet presentation. Reports (and this
codebase's own existing workarounds) indicate this can render stuck
centered on screen, non-draggable, with symmetric resize -- rather than
a proper attached, movable window -- unlike Linux/X11 where the same
dialog behaves as a normal draggable QDialog.
Two other dialogs in this codebase already explicitly opt into the
correct macOS sheet presentation for this exact reason:
ElementDialog::setUpWidget() and DiagramPropertiesDialog's setup both do
setWindowModality(Qt::WindowModal);
#ifdef Q_OS_MACOS
setWindowFlags(Qt::Sheet);
#endif
PropertiesEditorDialog -- used for editing Element, QetShapeItem, and
DiagramImageItem properties, all reached via double-click on a diagram
item -- had neither this opt-in nor an opt-out, so it likely fell into
the same automatic-sheet behavior but without the explicit flag,
matching the reported "stuck centered, can't drag" symptom.
Fix: apply the same setWindowModality()/Q_OS_MACOS Qt::Sheet pattern
already used by the other two dialogs, in PropertiesEditorDialog's
constructor -- fixing all three call sites (element, shape, and image
property editing) at once, since they share this one dialog class.
Verified: clean rebuild, all three call sites (element.cpp,
qetshapeitem.cpp, diagramimageitem.cpp) recompiled and linked
successfully with no errors or warnings.
Not verified: the actual reported symptom is macOS/Cocoa-specific
window presentation behavior, which cannot be reproduced or confirmed
fixed in this Linux/Xvfb sandbox -- no macOS environment is available
here. Confidence rests on the exact same fix pattern already being
established and presumably working for two other dialogs in this
codebase for the identical class of problem.
NewElementWizard::createNewElement() creates and show()s a QETElementEditor
for the freshly-created part, but never calls raise()/activateWindow().
On the reporter's macOS setup, the wizard (a modal sheet/child of the main
window) closing right before the new editor is shown apparently leaves the
main window as the active/key window, so the new editor window is created
but stays behind it -- and, being neither key nor frontmost, it also never
surfaces in the Dock's window list or the app's own Windows menu. This
matched the report exactly: the reporter saw the wizard finish with
seemingly no result, when in fact a new part genuinely was created and its
editor genuinely was opened, just hidden from view.
Fix: explicitly raise() and activateWindow() the new editor after show(),
so it becomes the frontmost/key window regardless of what state the wizard
leaves the main window in.
Verified: clean rebuild, only the intended object file recompiled and
linked successfully. Ran the full wizard flow live under Xvfb on Linux
(right-click user collection > "Nouvel élément" > through all 3 steps >
Finish) and confirmed the element editor opens correctly with a blank new
part, with no regression in the flow.
Not verified: the actual reported symptom is macOS-specific window-manager
behavior (key/frontmost window handling, Dock window-list registration),
which cannot be reproduced or confirmed fixed in this Linux/Xvfb sandbox --
no macOS or Wine-with-Cocoa environment is available here. raise()/
activateWindow() are the standard cross-platform Qt calls for this exact
problem and match the pattern already used elsewhere in the codebase
(QETApp::openElementLocations()'s already-open-editor branch), so
confidence rests on that precedent rather than a macOS-side confirmation.
XRefProperties::fromSettings() read the "xrefpos" QSettings key with no
default value. On a fresh install/project, the key doesn't exist yet, so
settings.value(...).toString() returns an empty string. QMetaEnum::keyToValue("")
returns -1 (invalid), which was then cast directly into m_xref_pos as
Qt::AlignmentFlag(-1) -- garbage, despite the class's own default
constructor documenting the intended default as Qt::AlignBottom.
This explains the reported symptom: dynamically generated cross-reference
text for master/slave-linked elements (e.g. magneto-thermal breaker,
thermal relay NC) rendered at an undefined position and overlapped the
element's own label, making the reference unreadable. The reporter's
manual workaround -- explicitly setting alignment to "Bottom" in Project
Properties > New Folio/Cross Referencing -- side-steps the bug precisely
by writing a valid "AlignBottom" value into QSettings, which fromSettings()
then reads back correctly on subsequent loads.
Fix: supply "AlignBottom" as the fallback default for the QSettings read,
matching the constructor's documented default and the reporter's
functioning workaround.
Verified: clean rebuild, only the intended object file recompiled and
linked successfully. Confirmed via a small standalone QMetaEnum test that
keyToValue("") returns -1/invalid while keyToValue("AlignBottom") returns
64 (== Qt::AlignBottom), reproducing the exact mechanism before the fix and
confirming the corrected default resolves to the intended value.
Not verified: a live before/after visual comparison of the rendered
cross-reference text position on an actual magneto-thermal/thermal-relay
diagram (would require constructing a multi-folio project with linked
master/slave elements and comparing label geometry, which was out of
scope for the time available). Confidence rests on the QMetaEnum
mechanism being unambiguous and the fix being a one-line default-value
correction with no other code path affected.
QETDiagramEditor::openBackupFiles() deleted the just-constructed
QETProject when it failed to reach ProjectState::Ok, but had no
continue/else after the delete - so addProject(project) ran
unconditionally on the now-dangling pointer, and addProject()
immediately dereferences it (new ProjectView(project), etc.).
This matches the report exactly: clicking Cancel on the restore-files
dialog (which just deletes the stale markers directly, never calling
openBackupFiles()) works fine, while clicking OK crashes whenever any
listed backup fails to open cleanly. Because the crash happens mid-
loop, cleanup for that file (and any later ones in the same batch)
never completes, which also explains the reporter's second complaint
that the restore list kept growing across sessions.
Fix: add the missing `continue` so a failed project is skipped
instead of being passed use-after-free to addProject().
Verified: clean rebuild, only the intended object file recompiled
and linked successfully. I attempted a live repro by crafting a
malformed stale-file marker to force ProjectState != Ok and clicking
OK under Xvfb, but this local build links against real KDE Frameworks
(BUILD_WITH_KF5=ON, confirmed via CMakeCache.txt) rather than the
in-tree nokde/kautosavefile.cpp reimplementation I initially targeted,
which uses a different marker directory/naming scheme
(~/.local/share/stalefiles/<app>/ via real KF5::KAutoSaveFile) that
I wasn't able to reverse-engineer well enough in the time available
to produce a matching malformed marker. Confidence in the fix instead
rests on the code being an unambiguous, textbook use-after-free (this
exact object is deleted on the line immediately above the missing
continue) with a single-line, side-effect-free fix.
Fix crash changing dynamic text color and confirming with Enter (bugtracker #323)
Works like charm:
- color is updated immediately
- no additional errors or warnings
- no crash anymore!
RotateTextsCommand::undo()/redo() called cti->forceMovedByUser(...)
instead of cti->forceRotateByUser(...) for ConductorTextItem entries
- a copy-paste mix-up between the two parallel user-override flags
that track independently whether a conductor's text was manually
moved vs manually rotated.
Because rotate_by_user_ was never actually set to true, the rotation
attribute-writing gate in Conductor::toXml() (which checks
wasRotatedByUser()) never fired, so a manual rotation applied via
"Orienter les textes" (Edit > Orienter les textes / Ctrl+Space) was
silently dropped on save: the rotation displayed correctly until the
project was closed and reopened, at which point it reverted to
default orientation.
Fix swaps both calls to forceRotateByUser(...), matching what the
constructor reads via wasRotatedByUser() when building m_cond_texts.
Verified: clean rebuild (506/506, no new warnings). Live-verified
under Xvfb that RotateTextsCommand's rotation correctly animates and
applies to ConductorTextItem text (confirmed via the "Orienter les
textes" dialog). A full save/close/reopen round-trip on a from-scratch
two-element wire was attempted but not completed due to unreliable
terminal-to-terminal wire drawing via synthetic mouse events in the
window-manager-less Xvfb sandbox; confidence in the fix instead rests
on tracing the exact save-gate code path (Conductor::toXml() gates
solely on wasRotatedByUser(), which the constructor/undo/redo all
already correctly reference elsewhere for the parallel
moved-by-user flag).
Bugtracker #309: selecting a result from the Search/Replace hit list
highlights the matching element, but on a diagram too large to fit
the current view, the view itself never scrolls -- the highlighted
element can be entirely off-screen with no indication of where it
went. The reporter pinpointed the exact spot,
searchandreplacewidget.cpp:1022, and suggested repositioning the
view's scrollbars.
SearchAndReplaceWidget::on_m_tree_widget_currentItemChanged() already
calls setHighlighted()/setSelected() on the matched element, text, or
conductor when a hit is selected; it just never brings it into view.
Added a call to QGraphicsItem::ensureVisible() alongside each of the
three existing highlight/select calls, so the view scrolls the
minimum needed for the match to be visible.
Followed the same approach as JumpToElementDialog's
activateCurrentItem() (added this session for #676) rather than
computing scrollbar positions by hand as suggested: ensureVisible()
scrolls every view showing the diagram automatically, works correctly
if a folio is open in more than one window, and needed no lookup of
which QGraphicsView the widget is attached to -- this widget doesn't
currently hold one. It was the only existing precedent for this exact
"scroll to reveal a matched item" problem anywhere in the codebase.
Verified live: built and ran the app under Xvfb, zoomed into one
corner of an example diagram until it needed scrollbars, searched for
text appearing in two different elements ("Offset null", both
op-amp offset-null pins), and confirmed selecting each result
scrolled the view to a different part of the diagram, centering the
matched element's highlight circle in the visible area each time. No
new build warnings.
plc-user on PR #693: the crash is fixed, but the color/font field and
the on-diagram text no longer update until you leave the properties
list and click in the diagram -- previously it updated as soon as you
clicked OK.
That's a side effect of the crash fix itself. The old, crashing code
returned a *live* QColorDialog as the item view's editor; clicking its
OK button called accept()/hide() on it, and hiding the active editor
happens to trip the base delegate's own focus-lost commit path -- so
the value applied immediately, racily, as a side effect of the same
mechanism that crashed on Enter. The fix (commit 4bd9b6b21) replaced
that with running the dialog synchronously inside createEditor() and
returning an inert placeholder with the result stashed in a property.
Correct for the crash, but it also removed that accidental commit
trigger: the placeholder never had focus to lose, so nothing tells
the view to read the value back until some unrelated interaction
(clicking away) incidentally triggers it.
Fix: explicitly emit commitData()/closeEditor() for the resolved
editor, deferred via QTimer::singleShot(0, ...) since the view only
registers createEditor()'s return value as "the active editor" after
createEditor() itself returns -- emitting synchronously, before
returning, would target a widget the view doesn't know about yet.
Applied to both font and color, since both share the exact same
"resolve synchronously in createEditor(), return an inert
placeholder" shape and thus the exact same gap; font just hadn't been
reported.
Verified with the same standalone harness from the crash fix (real
QTreeView + DynamicTextItemDelegate + QAbstractItemView::edit()),
this time deliberately *not* sending the synthetic Enter keypress the
crash-fix verification needed: clicks the dialog's real OK button,
lets the event loop run, and confirms the picked color lands in the
model on its own. Also reconfirmed the crash fix itself still holds
(clean exit, no synthetic-Enter needed either way now) and did a full
Release build (504/504) with no new warnings.
Weekly cron replaced with a monthly run (1st of each month, 02:00 UTC)
to reduce unnecessary CI load.
retention-days raised from 14 to 40 across all six artifact uploads
(Qt5 + Qt6 tracks) to cover the new monthly interval with a safety
margin -- 14 days was shorter than the gap between two cron runs,
so the latest build's artifacts could expire before the next one
replaced them.
When multiple elements are pasted or moved in one batch, each call to
autoBreakConductors() now receives the shared state from the previous
call. This prevents two elements in the same batch from independently
claiming the same conductor, which would result in a double-delete on
redo().
Requested by ispyisail in PR review.
plc-user