utils

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
WIP
WIP
Agreed. I seem to remember (this was a while ago now) that I found isAMacAndNotJS() and thought it might be an important distinction. Sounds like it's just redundant? Can tidy this another time then.

Agreed. I seem to remember (this was a while ago now) that I found isAMacAndNotJS() and thought it might be an important distinction. Sounds like it's just redundant? Can tidy this another time then.

Agree this is just touching on a much bigger task. However... The reason I /needed/ to do this rather than just /wanted/ to do this is that several of the classes I've been working on (e.g. jalview...

Agree this is just touching on a much bigger task.
However...
The reason I /needed/ to do this rather than just /wanted/ to do this is that several of the classes I've been working on (e.g. jalview.bin.Launcher, jalview.bin.HiDPISetting, jalview.bin.MemorySetting) run very early on (especially jalview.bin.Launcher!). This means Cache.log has perhaps not yet been initialised, so a Cache.log.debug doesn't log (unless you count reams of NullPointerExceptions as logging!).

In the case of HiDPISetting and MemorySetting that also get used in Getdown, where there is no jalview.bin.Cache, they currently have to use System.out and System.err [or maybe I could stub jalview.bin.Cache too]. I'd prefer them to use Cache.log when they can so this is an attempt at starting to decouple jalview.bin.Cache from other jalview things so it can be used standalone within Getdown. The main reason for wanting to do that is to have shared code to read the preferences between Jalview and Getdown.

I think there are lots of things that could be tidied up (particularly the overloading and additional logging functions via Cache which don't really reduce code at point of use, but are certainly u...

I think there are lots of things that could be tidied up (particularly the overloading and additional logging functions via Cache which don't really reduce code at point of use, but are certainly useful in spirit), but now is most definitely not the time to optimise and beautify code.

Ben Soares as far as I can see the only thing missing after this branch is merged to develop is this logic. I just did a quick test and it appears 'Platform.isAMac()' returns false under JalviewJS,...

Ben Soares as far as I can see the only thing missing after this branch is merged to develop is this logic. I just did a quick test and it appears 'Platform.isAMac()' returns false under JalviewJS, so probably not a dealbreaker. Do you agree ?

JAL-3608 cherry pick of 92cb745e7
JAL-3608 cherry pick of 92cb745e7
Ah it does run when a column selection is made in a split frame (and propagated to the other panel). Which seems to work! Maybe just needs a less clumsy AlignmentViewport.isColSelChanged() - this m...

Ah it does run when a column selection is made in a split frame (and propagated to the other panel). Which seems to work!
Maybe just needs a less clumsy AlignmentViewport.isColSelChanged() - this method seems to be trying to do two different things.

JAL-3390: Option to hide unmapped residues in structure viewer
JAL-3390: Option to hide unmapped residues in structure viewer
Suggesting renaming to PROPERTY_COLUMNS = "columns" as this is used to notify changes to column selection or visibility, not sequences

Suggesting renaming to PROPERTY_COLUMNS = "columns" as this is used to notify changes to column selection or visibility, not sequences

I think this code path rarely gets run (AlignmentViewport.notifySequence() rarely gets called). So what is it there for?

I think this code path rarely gets run (AlignmentViewport.notifySequence() rarely gets called).
So what is it there for?

Suggest renaming notifyAlignment() to notifyAlignmentChanged(), and notifySequence() to notifyColumnChange() as it it fired for changes to column (not sequence) selection or visibility.

Suggest renaming notifyAlignment() to notifyAlignmentChanged(), and notifySequence() to notifyColumnChange() as it it fired for changes to column (not sequence) selection or visibility.

Unclear what notifySequence() adds. As placed here, it is hardly ever reached.

Unclear what notifySequence() adds. As placed here, it is hardly ever reached.

Not sure how to review this (and the appletgui/js classes) without pointers to documentation and/or examples of how it is used.

Not sure how to review this (and the appletgui/js classes) without pointers to documentation and/or examples of how it is used.

Needs proper Javadoc

Needs proper Javadoc

Files added to doc folder: are this intended as working documents, or publishable documentation? Bearing in mind that that are findable on the web and a user might reasonably expect them to provide...

Files added to doc folder: are this intended as working documents, or publishable documentation? Bearing in mind that that are findable on the web and a user might reasonably expect them to provide documentation.
e.g. http://source.jalview.org/gitweb/?p=jalview.git;a=blob;f=doc/JalviewJS-startupParams.md;h=9ff33529b2dbc2da34d2a573ccf5d042194fbb0d;hb=refs/heads/Jalview-JS/develop

Due to the number of changes this is likely to be a 'best efforts' review.

Due to the number of changes this is likely to be a 'best efforts' review.

Commits to JS-develop from 2nd to 25th June 2020
Commits to JS-develop from 2nd to 25th June 2020
(Notes for self) - Ben verified on linux, OSX, Windows. Need to check there are docs and logging to indicate that the hard limit is reached, so if people want to adjust the limit they can !

(Notes for self) - Ben verified on linux, OSX, Windows.
Need to check there are docs and logging to indicate that the hard limit is reached, so if people want to adjust the limit they can !

JAL-3477 New sensible default decisions on memory, and new jvmmemmax setting for setting absolute...
JAL-3477 New sensible default decisions on memory, and new jvmmemmax setting for setting absolute...
Changing "... 8GB ..." to "... " + (noMemMaxHeapSizeDefault + 536870912) / 1073741824 + "GB ..." which does the trick! This should turn out to be the nearest integer number of GB being set (calcula...

Changing
"... 8GB ..."
to
"... " + (noMemMaxHeapSizeDefault + 536870912) / 1073741824 + "GB ..."
which does the trick!
This should turn out to be the nearest integer number of GB being set (calculated as (long)((mem+0.5GB)/1GB) ).

Just got to the comment about setting constant GIGABYTE. That makes much more sense and is much more readable

I agree the getdown (adapted) source that we use is not in the right place. Currently all getdown resources and built components get put under ./getdown/ to keep it a separate entity from Jalview. ...

I agree the getdown (adapted) source that we use is not in the right place. Currently all getdown resources and built components get put under ./getdown/ to keep it a separate entity from Jalview. I've created an issue detailing where the different getdown folders should be moved to (including putting ./getdown/src under ./src), but the build process for getdown is still necessarily separate from the Jalview build process. See issue JAL-3500.

A public class with no public members doesn't quite make sense. Should maybe have default (package) visiblity (or be an inner class of MemorySetting)? In fact, can't this just be another method in ...

A public class with no public members doesn't quite make sense. Should maybe have default (package) visiblity (or be an inner class of MemorySetting)?
In fact, can't this just be another method in MemorySetting?

Method javadoc please?

Method javadoc please?

Should the getdown code be in a jar file? And include MemorySettings? cf comment https://issues.jalview.org/browse/?focusedCommentId=21949&page=com.atlassian.jira.plugin.system.issuetabpanels:comme...

Should the getdown code be in a jar file? And include MemorySettings?
cf comment https://issues.jalview.org/browse/?focusedCommentId=21949&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#JAL-3278comment-21949

Not having getdown/data/Application on the project classpath confused me a bit (calls from Application to MemorySettings don't show up).

do this in getMemorySetting() instead ...cancel that...I see this is called from getdown/data/Application and either or both property might be null.

do this in getMemorySetting() instead
...cancel that...I see this is called from getdown/data/Application and either or both property might be null.

Declare type as List<String> is preferred style (refer to types by their interfaces wherever possible)

Declare type as List<String> is preferred style (refer to types by their interfaces wherever possible)

Compiler warning: loop label ARG is never referenced

Compiler warning: loop label ARG is never referenced