Skip to content

bug rollup: legend appearance, recording size, supress splash screen when display CLI help [CPP-754][CPP-758][CPP-737] #565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 21, 2022
Merged

bug rollup: legend appearance, recording size, supress splash screen when display CLI help [CPP-754][CPP-758][CPP-737] #565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6796ba3
Convert _handle_started to slot
john-michaelburke May 20, 2022
ba86029
Solution and Baseline legends + -> ● [CPP-758]
john-michaelburke May 20, 2022
42f691a
Prevent splash screen if displaying help[CPP-754]
john-michaelburke May 20, 2022
fe8d553
Incorrect default value for recording size[CPP-737]
john-michaelburke May 20, 2022
bc7a2fc
Respond to review requests.
john-michaelburke May 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions entrypoint/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,27 @@ fn main() -> Result<()> {
handle_wayland();
let current_exe = std::env::current_exe()?;
let parent = current_exe.parent().ok_or("no parent directory")?;
let mut command = std::process::Command::new(parent.join("swift-console-splash"));
match command.spawn() {
Ok(child) => {
let pid = child.id();
std::env::set_var(SWIFT_CONSOLE_PID, format!("{pid}"));
}
Err(e) => {
eprintln!("Starting splash screen failed: {e}");
}
};
let args: Vec<_> = std::env::args().collect();
let helper_found = args
.iter()
.any(|arg| matches!(arg.as_ref(), "--help" | "-h" | "--version" | "-V"));
if !helper_found {
let mut command = std::process::Command::new(parent.join("swift-console-splash"));
match command.spawn() {
Ok(child) => {
let pid = child.id();
std::env::set_var(SWIFT_CONSOLE_PID, format!("{pid}"));
}
Err(e) => {
eprintln!("Starting splash screen failed: {e}");
}
};
}

std::env::set_var("SWIFTNAV_CONSOLE_FROZEN", parent);

std::env::set_var("PYTHONHOME", parent);
std::env::set_var("PYTHONDONTWRITEBYTECODE", "1");
let args: Vec<_> = std::env::args().collect();
let exit_code = Python::with_gil(|py| {
let args = PyTuple::new(py, args);
let snav = py.import("swiftnav_console.main")?;
Expand Down
2 changes: 1 addition & 1 deletion resources/BaselineTabComponents/BaselinePlot.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ Item {
Label {
id: marker

text: "+ "
text: "● "
font.pixelSize: (Constants.mediumPixelSize + Constants.commonLegend.markerPixelSizeOffset)
font.bold: true
color: Constants.baselinePlot.colors[index]
Expand Down
2 changes: 1 addition & 1 deletion resources/LoggingBar.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ Rectangle {
if (recSize > 0)
recordingSize.text = bytesToString(recSize);
else
recordingSize.text = "0.00 MiB";
recordingSize.text = "0.00 MB";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MiB and MB are slightly different right? are we currently recording MBs and this is fixing the label?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see this is just for the 0 bytes recorded case. either way looking at bytesToString const k = 1024 should probably be 1000 if we want MBs not MiBs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this was just mislabelled, our calculation is converting to KB/MB/GB/etc:

swift-toolbox/resources/LoggingBar.qml

Line 247 in fe8d553

const k = 1024;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah so I think it should be 1000 (https://en.wikipedia.org/wiki/Megabyte table on the left is without the "i"s and it's powers of 10 not 2)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if the old console is doing 1024 and showing MBs maybe it's best just to forget about it hahaha

Copy link
Contributor

@silverjam silverjam May 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, technically MiB is supposed to be 1024 and MB is supposed to be 1000-- but in common usage MB can mean 1024 or 1000 depending on context and the apps team's argument was that most "end users" are not going to know what MiB is and in our context, we want to use 1024, not 1000...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh makes sense

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came here to say that MB is powers of 10 and MiB is powers of 2, as @silverjam mentioned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like powers of 2 were used in the old console but with the naming convention of KB/MB. So I'll just keep it as is then.

Copy link
Contributor

@silverjam silverjam May 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://devblogs.microsoft.com/oldnewthing/20090611-00/?p=17933

https://answers.microsoft.com/en-us/windows/forum/all/microsoft-to-clear-kib-vs-kb-etc-confusion-anytime/815f5e8f-510f-4bef-9f6c-26d723fe4eeb

I suppose when Windows finally conforms to the standard, then we should too.

}
}
}
Expand Down
2 changes: 1 addition & 1 deletion resources/SolutionTabComponents/SolutionPositionTab.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ Item {
Label {
id: marker

text: "+"
text: "● "
font.pixelSize: (Constants.mediumPixelSize + Constants.commonLegend.markerPixelSizeOffset)
font.bold: true
color: Constants.solutionPosition.colors[index]
Expand Down
1 change: 1 addition & 0 deletions swiftnav_console/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ def __init__(
else self._receive_messages_debug
)

@Slot() # type: ignore
def _handle_started(self):
QTimer.singleShot(0, self.receive_messages)

Expand Down