Toolbox Editor (1/2)

Refactored (many) functions related to creating a toolbox.
Can now:

Create a toolbox, create categories.
Switch between categories.
See the preview of your current toolbox.

The category tabs aren't being styled properly (but you can switch between categories).

This change is

Review status: 0 of 14 files reviewed at latest revision, 20 unresolved discussions.

src/factory_utils.js, line 1254 at r1 (raw file):

 * @return {!Array.<Blockly.Block>} Array with all of block's children blocks.
 */
FactoryUtils.getAllChildren = function(block) {

Maybe add a tagged low-priority issue to convert FactoryUtils to ES6?

src/factory_utils.js, line 1258 at r1 (raw file):

  childList.push(block);
  var children = block.getChildren();
  for (var i = 0, child; child = children[i]; i++) {

Nit: for...of since the index isn't used

src/controller/project_controller.js, line 87 at r1 (raw file):

  /**
   * Determines if a block loaded in the workspace has a definition (if it

Workspace contents instead of workspace? I think this function is only used in that context

src/controller/project_controller.js, line 93 at r1 (raw file):

   * @return {boolean} Whether block has been defined in project.
   */
  isDefinedBlock(block) {

seeing as this function just calls project.hasBlock, we could scrap it with the expectation that we just use project.hasBlock

src/controller/project_controller.js, line 181 at r1 (raw file):

   */
  addBlockDefinition(blockDef, libraryName) {
    this.project.getLibrary(libraryName).add(blockDef);

We'll have a conflict here but it's cool/small

src/model/block_library.js, line 173 at r1 (raw file):

   * @return {boolean} Whether or not blockType is stored in block library.
   */
  has(blockType) {

Same conflict situation

src/model/block_library_set.js, line 77 at r1 (raw file):

   */
  has(blockType) {
    return this.getLibrary(blockType) ? true : false;

Same as above (this is now named getBlockLibrary)

src/model/toolbox.js, line 107 at r1 (raw file):

   */
  setName(newName) {
    this.name = newName;

Can be removed//is in Resource

src/model/toolbox.js, line 207 at r1 (raw file):

  getElementById(id) {
    // From wfactory_model.js:getElementById(id)
    for (let i = 0; i < this.categoryList.length; i++) {

Same as previous for coment

src/model/toolbox.js, line 255 at r1 (raw file):

   */
  getSelectedId() {
    // From wfactory_model.js

reason to have this function instead of making call on result of getSelected?

src/model/toolbox.js, line 268 at r1 (raw file):

  getSelectedName() {
    // From wfactory_model.js:getSelectedName()
    return this.selected ? this.selected.name : null;

Same as above

src/model/toolbox.js, line 278 at r1 (raw file):

  getSelectedXml() {
    // REFACTOR: Moved in from wfactory_model.js
    return this.selected ? this.selected.xml : null;

Same as above

src/view/toolbox_editor_view.js, line 238 at r1 (raw file):

   * Display or hide the remove shadow button.
   * @param {boolean} show True if the remove shadow button should be shown, false
   *     otherwise.

random/low-priority query; will this method be private as well?

src/view/toolbox_editor_view.js, line 241 at r1 (raw file):

   */
  displayRemoveShadow(show) {
    // TODO: Move in from wfactory_model.js:displayRemoveShadow_(show)

Same as above

src/view/toolbox_editor_view.js, line 324 at r1 (raw file):

  /**
   * Determines if the workspace should be disabled. The workspace should be
   * disabled if category is a separator or has VARIABLE or PROCEDURE tags.

Missing param

src/view/toolbox_editor_view.js, line 365 at r1 (raw file):

   * Used to bind a click to a certain DOM element (used for category tabs).
   * Taken directly from code.js
   * @param {string|!Element} e1 Tab element or corresponding ID string.

the 'e1' vs 'el' thing (Nittiest of Nits™)

src/view/toolbox_editor_view.js, line 411 at r1 (raw file):

  markShadowBlocks(blocks) {
    // REFACTOR: Moved in from wfactory_view.js:markShadowBlocks(blocks)
    for (let i = 0; i < blocks.length; i++){

Same for loop thing

src/workspacefactory/wfactory_generator.js, line 57 at r1 (raw file):

};

 /**

Nit: also add a removed note to this file?

src/workspacefactory/wfactory_model.js, line 110 at r1 (raw file):

};

/**

Same as above

src/workspacefactory/wfactory_view.js, line 146 at r1 (raw file):

    };

/**

Same as above

Comments from Reviewable

Review status: 0 of 14 files reviewed at latest revision, 21 unresolved discussions.

src/factory_utils.js, line 1250 at r1 (raw file):

/**
 * Get all children of a block and add them to childList.

I don't think childList exists outside of this function// could change the description as a result

Comments from Reviewable

Review status: 0 of 14 files reviewed at latest revision, 22 unresolved discussions.

src/model/toolbox.js, line 427 at r1 (raw file):

   * @return {boolean} True if string is a used category name, false otherwise.
   */
  categoryIsInToolbox(name) {

since this is in toolbox.js, maybe we could change the name to hasCategory or some synonymous concept

Comments from Reviewable

Review status: 0 of 14 files reviewed at latest revision, 23 unresolved discussions.

src/view/toolbox_editor_view.js, line 284 at r1 (raw file):

   * and arrow buttons. Should be called when adding or removing elements
   * or when changing to a new element or when swapping to a different element.
   * TODO(evd2014): Switch to using CSS to add/remove styles.

we could move this todo to an issue and reference the number for concision//future development

Comments from Reviewable

Review status: 0 of 14 files reviewed at latest revision, 23 unresolved discussions.

src/factory_utils.js, line 1250 at r1 (raw file):
Previously, svouse wrote…
I don't think childList exists outside of this function// could change the description as a result

Done.

src/factory_utils.js, line 1254 at r1 (raw file):
Previously, svouse wrote…
Maybe add a tagged low-priority issue to convert FactoryUtils to ES6?

Done.

src/factory_utils.js, line 1258 at r1 (raw file):
Previously, svouse wrote…
Nit: for...of since the index isn't used

Done.

src/controller/project_controller.js, line 87 at r1 (raw file):
Previously, svouse wrote…
Workspace contents instead of workspace? I think this function is only used in that context

(Removed function.)

src/controller/project_controller.js, line 93 at r1 (raw file):
Previously, svouse wrote…
seeing as this function just calls project.hasBlock, we could scrap it with the expectation that we just use project.hasBlock

That is true. I removed isDefinedBlock().

src/model/toolbox.js, line 107 at r1 (raw file):
Previously, svouse wrote…
Can be removed//is in Resource

Removed.

src/model/toolbox.js, line 207 at r1 (raw file):
Previously, svouse wrote…
Same as previous for coment

I'll leave this here for now and remove it after your changes are merged into develop as well, since I don't see the getElementById() function in Resource now.
Also I think I'm not entirely sure how getElementById() is implemented at the Resource level because this function iterates through a field that is specific to Toolbox and isn't inherited from Resource. I don't think any of the other sub-project resources use ListElements?

src/model/toolbox.js, line 255 at r1 (raw file):
Previously, svouse wrote…
reason to have this function instead of making call on result of getSelected?

I think they had multiple getSelected type methods to make the fields seem more private? Though just using getSelected() would also do the same thing.

src/model/toolbox.js, line 427 at r1 (raw file):
Previously, svouse wrote…
since this is in toolbox.js, maybe we could change the name to hasCategory or some synonymous concept

True. Renamed to hasCategory().

src/view/toolbox_editor_view.js, line 238 at r1 (raw file):
Previously, svouse wrote…
random/low-priority query; will this method be private as well?

This is called from ToolboxController, so I don't think so 0:

src/view/toolbox_editor_view.js, line 241 at r1 (raw file):
Previously, svouse wrote…
Same as above

This is also called from ToolboxController?

src/view/toolbox_editor_view.js, line 284 at r1 (raw file):
Previously, svouse wrote…
we could move this todo to an issue and reference the number for concision//future development

Done.

src/view/toolbox_editor_view.js, line 324 at r1 (raw file):
Previously, svouse wrote…
Missing param

Added param.

src/view/toolbox_editor_view.js, line 365 at r1 (raw file):
Previously, svouse wrote…
the 'e1' vs 'el' thing (Nittiest of Nits™)

Good catch, though. :)

src/view/toolbox_editor_view.js, line 411 at r1 (raw file):
Previously, svouse wrote…
Same for loop thing

Done.

src/workspacefactory/wfactory_generator.js, line 57 at r1 (raw file):
Previously, svouse wrote…
Nit: also add a removed note to this file?

Wasn't sure what you meant by this comment, but I added a small comment indicating that functions were removed via refactor.

src/workspacefactory/wfactory_model.js, line 110 at r1 (raw file):
Previously, svouse wrote…
Same as above

Done.

src/workspacefactory/wfactory_view.js, line 146 at r1 (raw file):
Previously, svouse wrote…
Same as above

Done.

Comments from Reviewable

Review status: 0 of 15 files reviewed at latest revision, 16 unresolved discussions.

src/model/toolbox.js, line 207 at r1 (raw file):
Previously, chynu (celine choo) wrote…
I'll leave this here for now and remove it after your changes are merged into develop as well, since I don't see the getElementById() function in Resource now.
Also I think I'm not entirely sure how getElementById() is implemented at the Resource level because this function iterates through a field that is specific to Toolbox and isn't inherited from Resource. I don't think any of the other sub-project resources use ListElements?

Whoops this comment shouldn't be here my b

Comments from Reviewable

Review status: 0 of 15 files reviewed at latest revision, 17 unresolved discussions.

src/model/toolbox.js, line 209 at r1 (raw file):

    for (let i = 0; i < this.categoryList.length; i++) {
      let element = this.categoryList[i];
      if (element.id === id) {

Nit: we should try to be consistent about == vs === (although it high key doesn't really matter//if you're curious Rachel sent me this hip link to Neil's blog: https://neil.fraser.name/news/2017/06/07/)

Comments from Reviewable

Review status: 0 of 15 files reviewed at latest revision, 9 unresolved discussions.

src/model/toolbox.js, line 255 at r1 (raw file):
Previously, chynu (celine choo) wrote…
I think they had multiple getSelected type methods to make the fields seem more private? Though just using getSelected() would also do the same thing.

kk;we can keep them or ditch them if we want

src/workspacefactory/wfactory_generator.js, line 57 at r1 (raw file):
Previously, chynu (celine choo) wrote…
Wasn't sure what you meant by this comment, but I added a small comment indicating that functions were removed via refactor.

Yup; that's what I meant

Comments from Reviewable

Review status: 0 of 15 files reviewed at latest revision, 11 unresolved discussions.

src/factory_utils.js, line 1259 at r2 (raw file):

  var children = block.getChildren();
  for (let child of children) {
    FactoryUtils.getAllChildren(child, childList);

shouldn't this just have child as a param?

src/controller/editor_controller.js, line 123 at r2 (raw file):

   * @private
   */
  warnForUndefinedBlocks_() {

More logistical question: when would this issue a warning//how did those blocks get into the workspace?

src/model/toolbox.js, line 207 at r1 (raw file):
Previously, svouse wrote…
Whoops this comment shouldn't be here my b

I think I meant to refer to the for...of thing

Comments from Reviewable

Review status: 0 of 15 files reviewed at latest revision, 11 unresolved discussions.

src/factory_utils.js, line 1259 at r2 (raw file):
Previously, svouse wrote…
shouldn't this just have child as a param?

Oh... that is correct. Added a param.

src/controller/editor_controller.js, line 123 at r2 (raw file):
Previously, svouse wrote…
More logistical question: when would this issue a warning//how did those blocks get into the workspace?

For now I just copy-pasted it in from the old files, but I think that if a resource that is not BlockLibrary is imported by the user, there is a chance that some block within that resource may not be defined with a corresponding BlockDefinition.
In that case, we will probably give users a warning before they import it. That will give them the choice to define the block they are importing or ignore it and suffer potential errors/consequences.

src/model/toolbox.js, line 207 at r1 (raw file):
Previously, svouse wrote…
I think I meant to refer to the for...of thing

Ohh, I see. Done!

src/model/toolbox.js, line 209 at r1 (raw file):
Previously, svouse wrote…
Nit: we should try to be consistent about == vs === (although it high key doesn't really matter//if you're curious Rachel sent me this hip link to Neil's blog: https://neil.fraser.name/news/2017/06/07/)

I changed it to == to keep it consistent. It seems that Neil prefers == so that we are responsible for any strange equals errors?

src/model/toolbox.js, line 255 at r1 (raw file):
Previously, svouse wrote…
kk;we can keep them or ditch them if we want

Kept them for now since other files/references use these functions.

Comments from Reviewable

Review status: 0 of 15 files reviewed at latest revision, 7 unresolved discussions.

src/model/toolbox.js, line 209 at r1 (raw file):
Previously, chynu (celine choo) wrote…
I changed it to == to keep it consistent. It seems that Neil prefers == so that we are responsible for any strange equals errors?

I dig it//I'll update my '==='s to '==' as I encounter them

Comments from Reviewable

Review status: 0 of 15 files reviewed at latest revision, 7 unresolved discussions.

Comments from Reviewable

Reviewed 3 of 14 files at r1, 8 of 12 files at r2, 5 of 5 files at r3.
Review status: all files reviewed at latest revision, 15 unresolved discussions.

src/factory_utils.js, line 1250 at r3 (raw file):

/**
 * Get all children of a block and return as a list.

Note that this is recursive and will grab all descendents (grandchildren, etc.).

src/factory_utils.js, line 1256 at r3 (raw file):

 * @return {!Array.<Blockly.Block>} Array with all of block's children blocks.
 */
FactoryUtils.getAllChildren = function(block, childList) {

This looks very much like Block.getDescendants().

src/controller/editor_controller.js, line 133 at r3 (raw file):

    const project = this.projectController.getProject();
    for (let block of blocks) {
      if (!project.hasBlock(block.type)) {

Should  that project method be renamed hasBlockDefinition(..)?

src/controller/toolbox_controller.js, line 184 at r3 (raw file):

      }
      defaultName = name;
    } while (this.view.toolbox.hasCategory(name));

TODO: Check validity (because these probably become filenames, later).

src/controller/toolbox_controller.js, line 414 at r3 (raw file):

   */
  onChange(e) {
    console.log('change!!!!!!');

Remove debug logging

src/view/block_editor_view.js, line 141 at r3 (raw file):

   * @param {!BlockEditorController} controller BlockEditorController which will
   *     manage changes within event listeners.
   */

Add @package

src/view/toolbox_editor_view.js, line 107 at r3 (raw file):

      addShadow: $('#button_addShadow').get(0),
      removeShadow: $('#button_removeShadow').get(0)
    };

I'm guessing you'll find duplicate buttons/ids between these and the workspace editor.  Double check they are unique.
Also, I'm guess you never treat the buttons as a set (e.g., iterating through them). I'm thinking removeButton makes more sense than buttons.remove.  Personal preference.

src/view/toolbox_editor_view.js, line 309 at r3 (raw file):

   * TODO(#128): Switch to using CSS to add/remove styles.
   * @param {number} selectedIndex The index of the currently selected category,
   *   -1 if no categories created.

double indent when continuing a line.

src/workspacefactory/wfactory_generator.js, line 57 at r1 (raw file):
Previously, svouse wrote…
Yup; that's what I meant

These are transient comments, to be deleted at the end of the refactor, right? (Probably the whole file, right?)

Comments from Reviewable

Reviewed 2 of 12 files at r2, 2 of 5 files at r3.
Review status: all files reviewed at latest revision, 20 unresolved discussions.

src/factory_utils.js, line 1273 at r3 (raw file):

 * @return {string} ID of element shown.
 */
FactoryUtils.openModal = function(id) {

Handle the case that id is invalid. (null, or doesn't refer to anything)  Same below.  If document.getElementById(id) fails you will have a bad time.

src/factory_utils.js, line 1277 at r3 (raw file):

  document.getElementById(id).style.display = 'block';
  document.getElementById('modalShadow').style.display = 'block';
  return id;

Is this likely to be different from the input id?

src/view/toolbox_editor_view.js, line 278 at r3 (raw file):

  selectTab(id, selected) {
    // REFACTOR: Moved in from wfactory_view.js:setCategoryTabSelection(id, selected)
    if (!this.tabMap[id]) {

Put this.tabMap[id] in a variable at the beginning of the function, then use that variable everywhere below, to avoid extra lookups in your map.

src/view/toolbox_editor_view.js, line 365 at r3 (raw file):

   * @return {!Element} DOM element created for tab.
   */
  addCategoryRow(name, id) {

Are row and tab interchangeable?

src/view/toolbox_editor_view.js, line 422 at r3 (raw file):

  setBorderColor(id, color) {
    // From wfactory_view.js:setBorderColor(id, color)
    const tab = this.tabMap[id];

Check that tab exists.

src/workspacefactory/wfactory_view.js, line 69 at r3 (raw file):

};

// FUNCTIONS REMOVED FROM REFACTOR.

??

Comments from Reviewable

Reviewed 1 of 12 files at r2, 1 of 5 files at r3.
Review status: all files reviewed at latest revision, 29 unresolved discussions.

src/controller/toolbox_controller.js, line 83 at r3 (raw file):

  /**
   * Currently prompts the user for a name, checking that it's valid (not used

Is this comment still accurate?  What does "currently" refer to?

src/controller/toolbox_controller.js, line 88 at r3 (raw file):

  addCategory() {
    // From wfactory_controller.js:addCategory()
    // Transfers the user's blocks to a flyout if it's the first category created.

Does transferFlyoutBlocksToCategory check whether it's the first category created?

src/controller/toolbox_controller.js, line 94 at r3 (raw file):

    const name = this.promptForNewCategoryName('Enter the name of your new category:');
    if (!name) {  // Exit if cancelled.
      return;

Exiting after you've transferred blocks but before you've finished selecting the new category looks suspicious to me.

src/controller/toolbox_controller.js, line 119 at r3 (raw file):

    const category = new ListElement(ListElement.TYPE_CATEGORY, name);
    this.view.toolbox.addElement(category);
    // Create new category.

This comment seems redundant.

src/controller/toolbox_controller.js, line 321 at r3 (raw file):

      // Uses categories, creates a toolbox.
      if (!this.view.previewWorkspace.toolbox_) {
        tihs.reinjectPreview(tree); // Create a toolbox.

Misspelled this, which says to me that you haven't hit this branch in testing.

src/controller/toolbox_controller.js, line 341 at r3 (raw file):

   */
  generateToolboxXml() {
    const xmlDom = goog.dom.createDom('xml', {

@anmatanm for comments on use of createDom.

src/controller/toolbox_controller.js, line 348 at r3 (raw file):

    if (this.view.toolbox.isEmpty()) {
      // Toolbox has no categories.
      // this.loadToHiddenWorkspace_(this.view.toolbox.getSelectedXml());

Remove commented-out code.

src/controller/toolbox_controller.js, line 413 at r3 (raw file):

   * @param {!Event} e Change event in workspace.
   */
  onChange(e) {

Split this function up.  There's too much going on.

src/controller/toolbox_controller.js, line 416 at r3 (raw file):

    console.log('change!!!!!!');
    // Listen for Blockly move and delete events to update preview.
    // Not listening for Blockly create events because causes the user to drop

because ???? causes the user to drop blocks?
Also you're listening to change events.  You explicitly said you're listening to move and delete, and not to create, but haven't mentioned change at all.

Comments from Reviewable

Review status: 6 of 18 files reviewed at latest revision, 30 unresolved discussions, some commit checks failed.

src/factory_utils.js, line 1250 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
Note that this is recursive and will grab all descendents (grandchildren, etc.).

Done.

src/factory_utils.js, line 1256 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
This looks very much like Block.getDescendants().

Oooh. That is basically this function. I have removed this function and replaced any use of getAllChildren() with getDescendants().

src/factory_utils.js, line 1273 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Handle the case that id is invalid. (null, or doesn't refer to anything)  Same below.  If document.getElementById(id) fails you will have a bad time.

Done.

src/factory_utils.js, line 1277 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Is this likely to be different from the input id?

No, it is the same. I removed the return since it is probably not very useful.

src/controller/editor_controller.js, line 133 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
Should  that project method be renamed hasBlockDefinition(..)?

That is a good idea, considering there are many Block-seeming objects. Renamed.

src/controller/toolbox_controller.js, line 184 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
TODO: Check validity (because these probably become filenames, later).

Done.

src/controller/toolbox_controller.js, line 414 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
Remove debug logging

Done.

src/view/block_editor_view.js, line 141 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
Add @package

Done.

src/view/toolbox_editor_view.js, line 107 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
I'm guessing you'll find duplicate buttons/ids between these and the workspace editor.  Double check they are unique.
Also, I'm guess you never treat the buttons as a set (e.g., iterating through them). I'm thinking removeButton makes more sense than buttons.remove.  Personal preference.

Done.

src/view/toolbox_editor_view.js, line 278 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Put this.tabMap[id] in a variable at the beginning of the function, then use that variable everywhere below, to avoid extra lookups in your map.

Done.

src/view/toolbox_editor_view.js, line 309 at r3 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
double indent when continuing a line.

Done.

src/view/toolbox_editor_view.js, line 365 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Are row and tab interchangeable?

Yes. I renamed this to addCategoryTab() to be clearer--the function name kind of confused me, too.

src/view/toolbox_editor_view.js, line 422 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Check that tab exists.

Done.

src/workspacefactory/wfactory_generator.js, line 57 at r1 (raw file):
Previously, AnmAtAnm (Andrew n marshall) wrote…
These are transient comments, to be deleted at the end of the refactor, right? (Probably the whole file, right?)

Yes, by the end of refactor all of the old files should be removed.

src/workspacefactory/wfactory_view.js, line 69 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
??

Just to note that some functions within the file were removed while refactoring.

Comments from Reviewable

Reviewed 3 of 14 files at r1, 5 of 12 files at r2, 2 of 5 files at r3.
Review status: 6 of 18 files reviewed at latest revision, 36 unresolved discussions, some commit checks failed.

src/controller/toolbox_controller.js, line 432 at r3 (raw file):

    if (e.type == Blockly.Events.MOVE || (e.type == Blockly.Events.UI &&
        e.element == 'selected')) {
      const selected = Blockly.selected;

Why use blockly.selected instead of getting the selected item from the event?

src/controller/toolbox_controller.js, line 485 at r3 (raw file):

          this.view.buttons.addShadow.disabled = true;
        } else {
          // Selected block does not break any shadow block rules, but cannot

So what case does this cover?

src/controller/toolbox_controller.js, line 762 at r3 (raw file):

    // view.
    if (id != null) {
      console.log('id is not null in clearAndLoadElement()');

Remove logs

src/model/toolbox.js, line 211 at r3 (raw file):

   * object.
   * @param {number} index The index of the element to return.
   * @return {ListElement} The corresponding ListElement object in toolboxList.

, or null if no element existed at that index.

src/model/toolbox.js, line 296 at r3 (raw file):

  getCategoryId(name) {
    // From wfactory_model.js:getCategoryIdByName(name)
    this.categoryList.forEach((element) => {

This forEach looks like it'll have the same problem that we found with the ID foreach.

src/model/toolbox.js, line 376 at r3 (raw file):

  isShadowBlock(blockId) {
    // TODO: Move in from wfactory_model.js
    this.shadowBlocks.forEach((id) => {

Same problem with forEach.  Here and above, does that mean that you didn't hit this code in testing?

src/model/toolbox.js, line 421 at r3 (raw file):

    // From wfactory_model.js:hasCategoryByName(name)
    this.categoryList.forEach((element) => {
      if (element.type == ListElement.TYPE_CATEGORY && element.name == name) {

Same as above.

Comments from Reviewable

Reviewed 12 of 12 files at r4.
Review status: all files reviewed at latest revision, 35 unresolved discussions, some commit checks failed.

src/factory_utils.js, line 1252 at r4 (raw file):

 * Show a modal element, usually a dropdown list.
 * @param {string} id ID of element to show.
 * @return {string} ID of element shown.

Get rid of return annotation.

src/factory_utils.js, line 1257 at r4 (raw file):

  const modal = $('#' + id);
  if (!modal) {
    return null;

Just return;

src/factory_utils.js, line 1271 at r4 (raw file):

  const modal = $('#' + id);
  if (!modal) {
    return null;

Just return;

Comments from Reviewable

Review status: all files reviewed at latest revision, 32 unresolved discussions, some commit checks failed.

src/factory_utils.js, line 1252 at r4 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Get rid of return annotation.

Done.

src/factory_utils.js, line 1257 at r4 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Just return;

Done.

src/factory_utils.js, line 1271 at r4 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Just return;

Done.

src/controller/toolbox_controller.js, line 83 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Is this comment still accurate?  What does "currently" refer to?

Oh.. I'm not really sure, actually. I didn't do a great job of reading through all of the jsodocs I copy-pasted from the old code. /__\
Other than that the jsdoc is correct.

src/controller/toolbox_controller.js, line 88 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Does transferFlyoutBlocksToCategory check whether it's the first category created?

Yes. Would it be better design to check it within addCategory()?

src/controller/toolbox_controller.js, line 94 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Exiting after you've transferred blocks but before you've finished selecting the new category looks suspicious to me.

That is true. I changed the order so it potentially returns before transferring flyout blocks to a new category.

src/controller/toolbox_controller.js, line 119 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
This comment seems redundant.

Removed.

src/controller/toolbox_controller.js, line 321 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Misspelled this, which says to me that you haven't hit this branch in testing.

Ah, yes.. I fixed this in another branch but forgot to change it here. Done.

src/controller/toolbox_controller.js, line 348 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Remove commented-out code.

Done.

src/controller/toolbox_controller.js, line 413 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Split this function up.  There's too much going on.

Done.

src/controller/toolbox_controller.js, line 416 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
because ???? causes the user to drop blocks?
Also you're listening to change events.  You explicitly said you're listening to move and delete, and not to create, but haven't mentioned change at all.

Edited the comment. I didn't really read through all of the old comments very thoroughly, oops. :(

src/controller/toolbox_controller.js, line 432 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Why use blockly.selected instead of getting the selected item from the event?

I'm not entirely sure (this function was copy pasted with very minor reference changes). Would it be more reliable to use the event?

src/controller/toolbox_controller.js, line 485 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
So what case does this cover?

I'm not entirely sure (can't think of one) but I didn't really write this part so I can't say for sure that there is no such case (?).

src/controller/toolbox_controller.js, line 762 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Remove logs

Done.

src/model/toolbox.js, line 211 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
, or null if no element existed at that index.

Done.

src/model/toolbox.js, line 296 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
This forEach looks like it'll have the same problem that we found with the ID foreach.

Done.

src/model/toolbox.js, line 376 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Same problem with forEach.  Here and above, does that mean that you didn't hit this code in testing?

Shadow blocks haven't fully been implemented yet (I'm still debugging that part), and I'm not sure when exactly getCategoryById() is used (though I presume it might be for renaming the category/adding a color, which I haven't gotten to yet either).

src/model/toolbox.js, line 421 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Same as above.

Done.

Comments from Reviewable

Reviewed 4 of 4 files at r5.
Review status: all files reviewed at latest revision, 18 unresolved discussions, some commit checks failed.

src/controller/toolbox_controller.js, line 88 at r3 (raw file):
Previously, chynu (celine choo) wrote…
Yes. Would it be better design to check it within addCategory()?

Either is fine.  No strong opinion.

src/controller/toolbox_controller.js, line 432 at r3 (raw file):
Previously, chynu (celine choo) wrote…
I'm not entirely sure (this function was copy pasted with very minor reference changes). Would it be more reliable to use the event?

Yes, if the event has that information (I'm pretty sure it does).

Comments from Reviewable

Review status: all files reviewed at latest revision, 18 unresolved discussions, some commit checks failed.

src/model/toolbox.js, line 421 at r3 (raw file):
Previously, chynu (celine choo) wrote…
Done.

Hm, then some code is missing.  Reviewable is still showing a foreach.

Comments from Reviewable

Review status: 16 of 18 files reviewed at latest revision, 18 unresolved discussions, some commit checks failed.

src/controller/toolbox_controller.js, line 432 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Yes, if the event has that information (I'm pretty sure it does).

Changed to using the event.

src/model/toolbox.js, line 421 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
Hm, then some code is missing.  Reviewable is still showing a foreach.

Oops.. done (again).

Comments from Reviewable

 but let Andrew get one more look at it.

Reviewed 1 of 2 files at r6.
Review status: 17 of 18 files reviewed at latest revision, 14 unresolved discussions, some commit checks failed.

Comments from Reviewable

Reviewed 2 of 12 files at r4.
Review status: 17 of 18 files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

src/controller/toolbox_controller.js, line 341 at r3 (raw file):
Previously, rachel-fenichel (Rachel Fenichel) wrote…
@anmatanm for comments on use of createDom.

Good catch... Avoid using the second argument of creatDom(..). It's very broken on modern browsers, especially with XML (instead of XML). You can use goog.dom.setAttributes(..), but honestly the equivalent native DOM calls are pretty simple.

Comments from Reviewable

Review status: 17 of 18 files reviewed at latest revision, 11 unresolved discussions, some commit checks failed.

Comments from Reviewable


pull request to sit-rep_zh adding May08 report CHNsimplified translation first draft

Description of proposed changes
What is the goal of this pull request? What does this pull request change?
Related issue(s)

some previously used translations modified.
needing supervision
Related to #
Testing
What steps should be taken to test the changes you've proposed?
If you added or changed behavior in the codebase, did you update the tests, or do you need help with this?
Thank you for contributing to Nextstrain!

Hey @ZenithFZH, thanks for switching over the PR. Unfortunately it looks like we're running into all sorts of merge conflicts -- did you merge the master branch into sit-rep_zh?
I would recommend copying and pasting your translated file in a safe place, re-cloning the entire translations repo, checking out sit-rep_zh branch, copying your translated file into sit-rep_zh branch, and then retrying the pull request from your sit-rep_zh branch into Nexstrain's.
Sorry I know this sounds like a crazy hassle, but it gets better with use I promise!


Fix MSVC warnings for unknown escape character sequence

warning C4129: 'e': unrecognized character escape sequence

I'm not sure what the intention here is:

Accidental inclusion of \error: there are other cases in this file of error without \error
Use of GCC extension escape sequence \e
Wanting to print \ to the stream, ie. \\error

I went with the first, but let me know

Let's see if I'm right:
@swift-ci please test

@swift-ci please smoke test


hero mdbook bug: table of contents can't go to H3

Situation

hero mdbook bug: table of contents can't go to H3

How to reproduce

write a file with a table of contents, which goes to H3. The mdbook file rendered by hero contains the H3 level but not the table of contents.

the original markdown file toc is the following

the original markdown file in preview mode is the following

the hero markdown file rendering is the following

the code of this rendered file is the following

the headings are OK in the text (see H3, H4, ...)

Conclusion
The issue is how the table of contents is parsed. There are tabs missing to get a different heading level.

I'm a bit confused. i tried to check the indentation level of list items and it works fine. what is the issue with headers?
I know screenshots are useful, but can you provide the actual steps to reproduce so that i can redo them on my machine? (like the command you ran and on which input)

@MarioBassem Gotcha.
So steps:

clone repo: https://git.ourworld.tf/tfgrid/info_tfgrid in ~/code/git.ourworld.tf/tfgrid (should be root, sudo su)
cd into info_tfgrid
build the book with mdbook: hero mdbook -p $(pwd)/heroscript/test
Go to the hero mdbook directory built: cd ~/hero/var/mdbuild/test
Serve the book: mdbook serve --port 3003
You can see that the table of contents doesn't allow for a "3rd" level

The code is the following:

Table of Contents

- [Introduction](#introduction)
- [Prerequisites](#prerequisites)
- [Creating a New mdbook](#creating-a-new-mdbook)
  - [Cloning the mdbook Repository](#cloning-the-mdbook-repository)
  - [Directory Tree Example](#directory-tree-example)
  - [Files and Directories Example](#files-and-directories-example)
    - [books](#books)
    - [collections](#collections)
    - [heroscript](#heroscript)
    - [LICENSE](#license)
    - [README](#readme)
- [Generating the Mdbook Locally](#generating-the-mdbook-locally)
- [Pushing the Changes](#pushing-the-changes)
- [Generating the Mdbook from a Remote Repository](#generating-the-mdbook-from-a-remote-repository)

and the output should be:

@Mik-TF thanks.
i traced this, and the problem here is in the number of whitespace characters used. if you stick with tabs instead of whitespaces your problems will go away.
but in general we calculate the indentation level by taking the ceil of the number of whitespaces divided by 4. this means that 0 whitespaces is at level 1, [1,2,3,4] at level 2, [5,6,7,8] at level 3, and so on.

Yes I understand but mdbook's TOC create this TOC automatically so I can't change it.
I understand that hero mdbook parses differently than mdbook/markdown then. Anything we can do?

refactored list parsing to handle this case correctly


stdWindow - Improvements

Change window owner
SetWindowLongPtr(GWL_HWNDPARENT, hwnd) can be used to change the owner of a window. If SetWindowLongPtr(GWL_HWNDPARENT,0) is used the owner is affectively removed, and the window is treated as it's own "top level application" in a sense. Where as SetWindowLongPtr(GWL_HWNDPARENT, Application.hwnd) will make the window only appear when Application is also visible.
Show menu bar buttons
Example:
Call SetWindowLong(hform, GWL_STYLE, existing Or WS_MINIMIZEBOX)
Call DrawMenuBar(hform)

Set icon of window
hIcon = Form.Image1.Picture.Handle
Call SendMessage(hwnd, WM_SETICON, ICON_SMALL, ByVal hIcon)
Call SendMessage(hwnd, WM_SETICON, ICON_BIG, ByVal hIcon)

Enable addition and removal of window from TaskBar maybe more relevant for stdCOM...
See https://www.mrexcel.com/board/threads/display-userform-in-taskbar-with-custom-icon-and-hide-excel-mimicking-a-standalone-application.1123368/page-2
    Call CLSIDFromString(StrPtr(IID_PropertyStore), tIID)
    If SHGetPropertyStoreForWindow(hform, tIID, pPstore) = S_OK Then
        Call CLSIDFromString(StrPtr(CLSID_TASKLIST), tClsID)
        Call CLSIDFromString(StrPtr(IID_TASKLIST3), tIID)
        If CoCreateInstance(tClsID, 0, CLSCTX_INPROC_SERVER, tIID, pTBarList) = S_OK Then
            SetProp Application.hwnd, "pTBarList", pTBarList
            Call vtblCall(pTBarList, ITASKLIST3_HrInit, vbLong, CC_STDCALL) 'HrInit Method
            Call vtblCall(pTBarList, ITASKLIST3_DeleteTab, vbLong, CC_STDCALL, hform) 'DeleteTab Method
            Call vtblCall(pTBarList, ITASKLIST3_AddTab, vbLong, CC_STDCALL, hform) 'AddTab Method
            Call vtblCall(pTBarList, ITASKLIST3_ActivateTab, vbLong, CC_STDCALL, hform) 'ActivateTab Method
            If Len(ThumbnailTooltip) Then
                Call vtblCall(pTBarList, ITASKLIST3_SetThumbnailTooltip, vbLong, CC_STDCALL, hform, StrPtr(ThumbnailTooltip)) 'ActivateTab Method
            End If
        End If
    End If

Hi. I'm not used to github :|) so i don't know if this is the place to write. Did you ever get anywhere with the Hooking part of the stdWindow class?

Added HICON and stdImage:
Private Sub UserForm_Initialize()
  With stdWindow.CreateFromIUnknown(Me)
    Call .setOwnerHandle(0)
    .HICON = stdWindow.CreateFromHwnd(Application.VBE.MainWindow.hWnd).HICON
    .HICON = stdImage.CreateFromStdPicture(Image1.picture).HICON
    .HICON = stdImage.CreateFromShape(Sheet1.Shapes("Picture 2")).HICON
    .HICON = stdImage.CreateFromFile("C:\Users\sancarn\Pictures\yuumi.png").HICON
  End With
End Sub


ASUS ROG Gladius II Core

libratbag has support for similar devices such as Gladius II and Gladius II Origin. If you are interested in implementing this mouse, I can provide you any diagnostic data you need.

Hello, that would be nice!
Please, post the output of lsusb -vd 0b05: in the terminal.
Gentle ping, @kyokenn. Since you made the driver for ASUS devices, would you like to get mentioned whenever support for new devices for it is requested?
I also checked your rogdrv repository, this mouse doesn't seem to be supported there either.

Since you made the driver for ASUS devices, would you like to get mentioned whenever support for new devices for it is requested?

Yeah, sure.

@kyokenn, here is the output:
[evrifaessa@fedora ~]$ lsusb -vd 0b05:

Bus 001 Device 005: ID 0b05:18dd ASUSTek Computer, Inc. ROG GLADIUS II CORE
Couldn't open device, some information will be missing
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x0b05 ASUSTek Computer, Inc.
  idProduct          0x18dd
  bcdDevice            2.03
  iManufacturer           1 ASUSTeK
  iProduct                2 ROG GLADIUS II CORE
  iSerial                 3
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x005b
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0xe0
      Self Powered
      Remote Wakeup
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0
      bInterfaceProtocol      0
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      34
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      1 Boot Interface Subclass
      bInterfaceProtocol      2 Mouse
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      66
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0006  1x 6 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0
      bInterfaceProtocol      0
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength     136
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0011  1x 17 bytes
        bInterval               1

I have added device file https://github.com/kyokenn/libratbag/blob/g2core/data/devices/asus-rog-gladius2-core.device
You can test it without rebuilding, if you already have the latest libratbag build with asus driver.

@kyokenn, I don't know if I have the ASUS driver. I don't have any ASUS .device files related to ASUS models in /usr/share/libratbag. I tried adding this device file, then restarting the ratbagd service, but Piper still can't detect my mouse.

I don't have any ASUS .device files related to ASUS models
I see. So you have an older version. You need to build the latest one. You can build from my branch.

@evrifaessa, the support for ASUS devices only came out very recently, in v0.17 released less than a week ago, so not all have updated yet. Since that may take a while, you should probably build libratbag yourself. https://github.com/libratbag/libratbag/wiki/Building-from-source.
To track whether libratbag in repository of your distro has been updated, I suggest using Repology: https://repology.org/project/libratbag/versions.

I'm using Fedora 37. I think I'll wait until the update arrives to me.

Would it be possible to just test it with rogdrv?

@evrifaessa, hi! Fedora 38 with libratbag v0.17 has been out for a while, if you've updated, could you check it now?

Just tested. Piper can now recognize the mouse. Though I can say there are a few issues.

It recognizes two different devices under the same name. A regular one and one with unknown button mappings.
LEDs are kind of buggy:

Solid colors won't work

The device has two RGB spots, not three

The DPI can go up to 6200, whereas Piper only lets you go until 600.
No image of the mouse exists. The default one is shown instead (the mouse holding a 404 sign)
If you remove two of the three profiles, leaving only one, the two profiles you've removed eventually come back with the default settings.

This is my initial testing. Let me know if I can help with any debugging.


Problem with ListView EdgeInset

Run the following app:

class MyHList extends StatelessWidget{
  final String title = "Horizontal List";
  @override
  Widget build(BuildContext context) {
    // TODO: implement build
    return new MaterialApp(
      title: title,
      home: new Scaffold(
        appBar: new AppBar(
          title: new Text(title),
        ),
        body: new Container(
          color: Colors.black,
          margin: const EdgeInsets.all(20.0),
          child: new ListView(
            scrollDirection: Axis.horizontal,
            children: <Widget>[
              new Container(
                width: 160.0,
                color: Colors.red,
              ),
              new Container(
                width: 160.0,
                color: Colors.blue,
              ),
              new Container(
                width: 160.0,
                color: Colors.green,
              ),
              new Container(
                width: 160.0,
                color: Colors.yellow,
              ),
              new Container(
                width: 160.0,
                color: Colors.orange,
              ),
            ],
          ),
        )
      ),
    );
  }
}

void main() => runApp(new MyApp());

class MyApp extends StatelessWidget{
  @override
  Widget build(BuildContext context) {
    return new MyHList();
  }
}

No problem with landscape

When I scrolling, It displayed wrong.

I was able to reproduce the problem: when the app is hot-restarted in landscape mode, the list's viewport is not inset by the 16.0 margin on the right. When the list is scrolled, the margin on the right appears.


Improved README.md

Thank you for your suggestion! However, I disagree that this improves the readme file. "Doc" is a valid abbreviation of document and we use the term docs consistently to refer the entire set of documentation for Kubeflow. To learn more about contributing to the Kubeflow docs, see the Kubeflow docs style guide.


Improve verbosity of panic messages in CsMat

This improves verbosity of some panic messages in CsMat.
Currently, on the following example,
use sprs::CsMat;

let indptr = vec![0, 5, 8];
let indices = vec![761698, 328290, 828689, 761698, 780185, 901149, 780185, 144749, 258307];
let data = vec![1, 1, 1, 1, 1, 1, 1, 1, 1];
let a = CsMat::new((2, 1000000), indptr, indices, data);

I'm getting an obscure Indptr length does not match dimension panic message, so this improves the verbosity of such messages a bit, to show the lengths.

rustfm is failing on the code I have not changed (and I did run cargo fmt prior to commiting).


Added QuoteNOption Component

Added QuoteNOption Component.
We need to enhance UI and Animations.

@suhasbansude Thank you for PR. Please go through our UI Designs and make appropriate changes.

@suhasbansude Can you please add a screenshot here??? It becomes easy for us to review. And please add the following

Create a new page as follows /pages/Quiz which will contain
a) Quiz.js
b) index.js
c) Quiz.css
Add a route to the page /RouteManager
Call the QuoteNOptionContainer from the Quiz page

@suhasbansude As I said earlier, Please go through our UI Designs. The color scheme in UI Design is different from what you have used. Also use green-tick.svg and red-cross.svg for answers and not star.svg. Once that is done the PR is ready to be merged.
If You have any doubts please let me know.

Also due to new PR merged, there are merge conflicts now, so please first merge existing develop  with your local copy to resolve the conflict.

Hey, @suhasbansude please add screenshot once you have made required changes to the UI. The PR is almost ready to be merged.

@twishasaraiya ok
I am currently working on UI.

Please check New UI Related changes

@suhasbansude Great work 👍. Thank so you much. Some changes would be required when linked to the backend but merging it now.


The abstract-decoratee check must be warning but not error

Consider the code which is get after unphi operation:
# This is the default 64+ symbols comment in front of named abstract object.
[] > a
  # This is the default 64+ symbols comment in front of named abstract object.
  [] > @
    # This is the default 64+ symbols comment in front of named abstract object.
    [] > @
      # This is the default 64+ symbols comment in front of named abstract object.
      [] > @
        for. > @
          malloc.
            eolang.
              org.
                Q
          number.:0
            eolang.
              org.
                Q
            00-00-00-00-00-00-00-00:0

It's valid and it works but it fails with error: Abstract object can't be used as a decoratee.
I believe this check must be a warning so it's kind of bad practice to write a code in a such way
@yegor256 WDYT?

@yegor256 I'm not sure since it breaks structure of original EO code which is for example a unit test with such design

@yegor256 yes, it helps, but still I think abstract-decoratee is not an error but rather bad way to write EO code. The same as sparse decoration

@maxonfjvipon OK, let's change its severity to warning

@yegor256 can I take this one?


Editing the website

When browsing the help (https://help.jabref.org/), clicking on the mascot at the upper right-hand corner leads to the edit page of the help. For example, if you are at https://help.jabref.org/en/FAQlinux , you go to https://github.com/JabRef/help.jabref.org/blob/gh-pages/en/FAQlinux.md .
This is not the case for the website (http://www.jabref.org/): it leads to the JabRef source code (https://github.com/JabRef/jabref) instead of  the website code (https://github.com/JabRef/www.jabref.org).
Maybe it is a feature, but I was surprised about it.

Going to the GitHub project is the usual setting for projects.
The idea behind the different links is:

www.jabref.org is a landing page for JabRef interested guys
all other pages are pages for people knowing more about JabRef.

Thus, corner link at help was that the more experienced users going there.
I can change everything to go to the GitHub project page as we have the "Edit this page" link at the bottom.

So it is a feature! ;-)

Personally, I would remove the icon from the main page completely. I don't think that an average user knows what github is and our page here is definitely not a good landing page for a new user. Just add the link to github under Contributing and the few guys that look for a coding opportunity still find us :-).
(By the way, I also don't like the Flattr button...it just don't fits into the design and gives no real benefit in my eyes.)
Can you also add a tooltip in the help pages to the github icon saying that this leads to the Edit page? And when we are at improvements, is it possible to change the link from

view on github: https://github.com/JabRef/help.jabref.org/blob/gh-pages/index.md
to
edit on github: https://github.com/JabRef/help.jabref.org/edit/gh-pages/index.md

The github link now moved to the footer.


[SPARK-50526][SS] Add store encoding format conf into offset log and block non supported stateful operators from using avro

What changes were proposed in this pull request?
Add store encoding format conf into offset log and block non supported stateful operators from using avro
Why are the changes needed?
Changes are needed to ensure that encoding format info is stored in offset log and that trying to use avro in a query that has stateful operators that don't support this encoding format yet will result in query failure.
Does this PR introduce any user-facing change?
Yes
How was this patch tested?
Added unit tests
[info] Run completed in 2 seconds, 547 milliseconds.
[info] Total number of tests run: 6
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 6, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 13 s, completed Dec 9, 2024, 6:15:25 PM

Was this patch authored or co-authored using generative AI tooling?
No

cc - @ericm-db @HeartSaVioR - PTAL, thx !

Shall we add the test for checking the default value? We can use the traditional approach with Spark 3.5 checkpoint.

Done - added the test

Thanks! Merging to master.


Update nullsafety version

Update plugin version because its breaking change.

No news on this merge? without null-safety (and v1.6.4-nullsafety.0 doesn't work) is a big problem. Thanks a lot

pull_to_refresh: 1.6.1-nullsafety.0
remember that we don't have '^' in version to avoid updating

Pre-releases of null-safety versions wont be updated untill you want it.  So untill you will write in pubspec
pull_to_refresh: ^2.0.0-nullsafety.1


[ClangImporter] restrict import of macros with typedef'ed casts

What's in this pull request?
Following the discussion in #3336: import only casts with builtin canonical types to make sure they can be typechecked.

Before merging this pull request to apple/swift repository:

[ ] Test pull request on Swift continuous integration.

Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
    Platform     | Comment
    ------------ | -------------
    All supported platforms     | @swift-ci Please smoke test
    All supported platforms     | @swift-ci Please smoke test and merge
    OS X platform               | @swift-ci Please smoke test OS X platform
    Linux platform              | @swift-ci Please smoke test Linux platform

A smoke test on macOS does the following:

Builds the compiler incrementally.
Builds the standard library only for macOS. Simulator standard libraries and
device standard libraries are not built.
lldb is not built.
The test and validation-test targets are run only for macOS. The optimized
version of these tests are not run.

A smoke test on Linux does the following:

Builds the compiler incrementally.
Builds the standard library incrementally.
lldb is built incrementally.
The swift test and validation-test targets are run. The optimized version of these
tests are not run.
lldb is tested.

Validation Testing
    Platform     | Comment
    ------------ | -------------
    All supported platforms     | @swift-ci Please test
    All supported platforms     | @swift-ci Please test and merge
    OS X platform               | @swift-ci Please test OS X platform
    OS X platform               | @swift-ci Please benchmark
    Linux platform              | @swift-ci Please test Linux platform

Lint Testing
    Language     | Comment
    ------------ | -------------
    Python       | @swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

Please add a test case as well.

Sure, you're welcome.

Looks good. Thanks, Ingmar!
@swift-ci Please test

Once more?
@swift-ci please test.

I'm going to say that first test got past the important things. Merging.


Removing task agnostic logging

Pull Review Summary
Description
Removing task agnostic logging added previously for debugging
Type of change

Bug fix (change which fixes an issue)
New feature (change which adds functionality)
This change requires a documentation update

Tests

Merged as a part of NER benchmarking refactor instead.


python312Packages.py-ocsf-models: init at 0.1.1

OCSF models in Python using Pydantic
https://github.com/prowler-cloud/py-ocsf-models
Needed for prowler >= 4.3.7
Description of changes

Things done

Built on platform(s)

[ ] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)

[ ] sandbox = relaxed
[ ] sandbox = true

[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[ ] Fits CONTRIBUTING.md.

Add a :+1: reaction to pull requests you find important.

Result of nixpkgs-review pr 345373 run on x86_64-linux 1

4 packages built:

python311Packages.py-ocsf-models
python311Packages.py-ocsf-models.dist
python312Packages.py-ocsf-models
python312Packages.py-ocsf-models.dist


Please provide a new setting: verticalScrollBarWidth

Please provide an option in the config.json to:

set the width of the vertical scrollbar to some fixed width - for e.g.: 32px
there should be something in the config like: "verticalScrollBarWidth" or something similar

Why should users use this?
The new behaviour of the vertical scrollbar in windows terminal provides no advantage. It's confusing the users.
Many users want to use windows terminal as replacement for the dos box and this new behaviour drives them crazy.
Example:
"defaults":
{
"scrollBarState" : "visible",
"verticalScrollBarWidth" : 32
}
... and this will set the vertical scrollbar to a fixed width

This makes sense to group with the rest of the "theming" asks in #3327. Thanks!

I need this as well, because it is too slim and this makes it difficult to click it with the mouse pointer.

As used in this script: https://github.com/mhgschmidt/w10install/blob/master/scripts/modify-desktoptheme.cmd
Config for Aerolite is here: https://github.com/mhgschmidt/w10install/blob/master/scripts/config/aerolite.theme
Works for me. Givin up and closing this ticket.

@mhgschmidt please leave it open. I don't want to alter the theme.

if you don't want to get notifications of this issue, you can unsubscribe from it. but there are others who don't want the theme workaround. and it's a workaround, not a solution to the issue you opened

Reopened.
@zadjii-msft : This looks like an easy and small task for me. Can you do something here ?

If you'd like to contribute, we'd happily review the PR. I'm not sure there's an easy way to change the width of a XAML scrollbar, but if there is, this would be the place to start:
https://github.com/microsoft/terminal/blob/e5293b78143bdbb7bff4f83fadd5cf5c080608e9/src/cascadia/TerminalControl/TermControl.xaml#L92-L105

lmao it works but looks insane:

@StephenLPeters (sorry you're my go to "winui on GH person" now) Any ideas if there's an easy way to set the thickness of the thumb too? This obviously looks insane. At least the whole track works for hit testing, though, the up/down buttons don't.

Note to self: <x:Double x:Key="ScrollBarSize">16</x:Double> might be the key I'm looking for.

@zadjii-msft this is the resource that drives this value: https://github.com/microsoft/microsoft-ui-xaml/blob/e4c5d3443caa0242be0953010dd5ab3ddb4c2925/dev/CommonStyles/ScrollBar_themeresources.xaml#L785 unfortunately it is a StaticResource defined in a Non-ThemeDictionary in Winui2 which, because of some... quirks... in the resource lookup system means that you will not be able to override this propety from Terminal.  Therefore I think the only option you have is to retemplate the control.  We could/should fix this in winui2 and make it a ThemeResource which would allow you to override it in the visual tree.

@StephenLPeters thanks for the follow up. Seems to me like the best course of action would be:

Provide this setting now, under the experimental. namespace, because it looks a little silly but works
Use https://github.com/microsoft/microsoft-ui-xaml/issues/6684 to drive changing the VerticalThumb Width to a ThemeResource.
When there's a version with the above fix in it available for the Terminal to ingest, we remove the experimental. bit from the setting and use that themeresource to fix the appearance.

Yes, although we should confirm that this resource is the only one that you need access to in order to fulfill your design. Its possible that you'd also need to change the roundedness of the corners or the margin from the edge of the well, or something like that.  Maybe we can pull the nuget package from our PR pipeline and test that terminal is able to make the changes you need. I'll make the PR now.

Team discussion:

yeeting a experimental feature into the OS as a bugfix is not the right move, especially one that we don't have runway to remove the experimental aspect before snapping the last OS build.
We should just patch the scrollbar width for what it used to be. That would be the simplest fix......

Oh look at that. He just fixed it like that, because he's the best. https://github.com/microsoft/microsoft-ui-xaml/pull/6686. Thanks!
That might change our previously discussed plans. More discussion to follow tomorrow, I'm sure.

I suspect there is more that is needed to make this look correct.. for instance the chevrons are also miss sized so we'll need to figure out the set of resources that are needed.

I'm sorry, I need to downvote this.  I completely agree that the scrollbars are too thin, but we really should not be configuring scrollbars separately for every app.  Scrollbars have gotten too thin everywhere (Feedback hub, Window News, Xbox apps, every single Electron app, the list goes on.)  This really needs to be pushed up to some Microsoft UI team so we can control this globally at the OS level for all apps.

until you find the right place to file the issue with microsoft, downvoting this helps no one

Actually, you know what? I'm inclined to agree with @jvincent-siplay. No other application in common use today offers this kind of customization--if it would, does it offer a switch for making the buttons smaller or larger? Or making the status bar taller or shorter? The caption controls smaller, redder, or more translucent?--and so I don't think we should either. I would say that downvoting this issue has helped us, as I think it's going to clear this thing off our backlog that shouldn't be on it.
The Terminal team is able to funnel these feature requests to the right people, but we have no special ability to make sure they get acted on.

no application offers customization

the problem is microsoft terminal is a rebel citizen in windows 10
see how other native microsoft apps have their scrollbars in windows 10:
device manager, command prompt, explorer, task manager, edge, uwp settings app

what all these scrollbars have i common? they're all usable. they're usable interactive ui elements, not just page progress indicators.
if you call customization the ability to be usable... then I am left with no other argument.

too damn hard to grab even notice the where the scroll bar is at .. lol

too damn hard to grab even notice where the scroll bar is at .. lol
while title bar is wide as ocean. Try to find the scroll bar.

Why can't this version be default? It's soo much better.

I presume that the team/person in charge has prioritized visibility/aestethics over usability.
They must think "the bar is just an indicator, nobody actually clicks it to scroll, everyone simply usees the mouse wheel for scrolling".
The scary thought is however, "what if they guessed right, and we - users of precision click and drag on scrollbar - are now just a minority?"

Why can't this version be default? It's soo much better. But only shows up when you hover on the thin line.
[screenshot]

FWIW, that was added in #14047, and is shipping in 1.17.

We also changed the width of the Terminal's scrollbar to align with the rest of the OS's scrollbar's in #12608, back in 1.14. We're a WinUI-style scrollbar, with a ComCtl-style width now.

They must think "the bar is just an indicator, nobody actually clicks it to scroll

🤔
In general, the Terminal repo isn't the right place for feedback on the design of the WinUI scroll bars. A better place for your feedback would be: https://github.com/microsoft/microsoft-ui-xaml/discussions/7422

They must think "the bar is just an indicator, nobody actually clicks it to scroll, everyone simply usees the mouse wheel for scrolling".
The scary thought is however, "what if they guessed right, and we - users of precision click and drag on scrollbar - are now just a minority?"

I also use scroll bar to track where am I in the jungle of command line output.

FWIW, that was added in https://github.com/microsoft/terminal/pull/14047, and is shipping in 1.17.

Thanks! Eagerly waiting for the release then.

https://github.com/microsoft/terminal/issues/15113

With Windows Terminal v1.12.1073, the scrollbar looks like this:

however starting with the next version, Windows Terminal v1.12.1098, the scrollbar is narrow:

and in future versions even rounded at the corners. I find these changes distracting. Also, they are different from my browser Mozilla Firefox:

My editor Vim:

and even Windows Explorer:

this is quite annoying, I wish Microsoft would just focus on making the terminal itself better, and not spend time on these "bikeshed" issues, especially in regards to changes that are difficult or impossible for users to revert. in my case I am now forced to use the old version forever.

Please consider visually impaired users too - the current indicator is also too small


IAM IdC cross-region support

Issue #, if available:
Closes #59
Description of changes:
Added deployment option to specify the IAM IdC region used by the gateway when making API calls to IdC. This region is provided to the Lambda functions so they can call the IdC instance in the appropriate region. Also modified the exchangeIdPTokenForIAMSessionCreds function to use this new configured region.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

The PR looks good to me. @james-jory Can you please confirm that you have tested the changes end to end?

Yes. I used the publish.sh script to stage the deployment assets to a bucket in my account, deployed the staged assets using the CFN link (i.e., the link for the "Launch stack" button but from my bucket), and tested the gateway authentication and functionality for a cross-region configuration.


Problem: routing url-encoded URLs doesn't work properly

The URL is being decoded before any matching is done.
So, for example, /hello/%3Fa will not match /hello/a because %3F
is already decoded to ?.
Solution: use raw URL to do the routing and decode individual parts
instead.

Thanks, looks good! Please also add am entry in the changelog and I'll merge.

Updated the changelog.

@tomaka Hi Pierre, what's the ETA for the next release? I am running into vendoring/frozen builds issues packaging my app for NixOS -- I am yet to see a workaround for git-based dependencies in their process. In the meantime, the only solid way for things to work is to depend on crates.io-only packages. Hence the question about the next release!
Thanks.

@yrashk If you submit a PR that bumps the version in Cargo.toml and CHANGELOG, it will automatically be published once merged.

Awesome, will do!
On Mon, Mar 5, 2018, 8:46 PM Pierre Krieger<EMAIL_ADDRESS>wrote:

@yrashk https://github.com/yrashk If you submit a PR that bumps the
version in Cargo.toml and CHANGELOG, it will automatically be published
once merged.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/tomaka/rouille/pull/173#issuecomment-370423520, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAABxOhEefQVn4GGArqldAo14JOMdVBFks5tbUG4gaJpZM4SOwzd
.


remove extra count imageset query

remove count imageset query to cut backend time in half

Signed-off-by: Jonathan Holloway<EMAIL_ADDRESS>Description
Removing extra imageset count query to cut backend time in half (currently 4 extra seconds)
Fixes # (issue) THEEDGE-1800
Type of change
What is it?

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Documentation update
[ ] Tests update
[ ] Refactor

Checklist:

[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] New and existing unit tests pass locally with my changes
[x] I run go fmt ./... to check that my code is properly formatted
[x] I run go vet ./... to check that my code is free of common Go style mistakes

Codecov Report

Merging #729 (c720f99) into main (ac156ed) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #729   +/-   ##
=======================================
  Coverage   31.24%   31.24%
=======================================
  Files          44       44
  Lines        4593     4583   -10
=======================================
- Hits         1435     1432    -3
+ Misses       2960     2954    -6
+ Partials      198      197    -1

Impacted Files
Coverage Δ

pkg/routes/imagesets.go
29.01% <ø> (-0.05%)
:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac156ed...c720f99. Read the comment docs.


Getting Slack auth token when user installs the bot

Is there any way to retrieve the auth token when someone installs the bot?

It's passed in ChannelData for every message

@craigjensen I want to get the token before user sends the first message.

Your bot should receive a conversationUpdate message when it's added to a channel.  The Api token is included in ChannelData for this event as well.

@craigjensen I receive the update only after I invite the bot to a channel. But I want to get the token before that so that I could execute a few slack APIs (e.g. sending a welcome message to users).

@craigjensen Ideally, the botframework console should allow developers to register an optional http webhook/callback that will be invoked after the slack oauth is completed.

Thanks for the feedback.  I opened a feature request to internally track this going forward.  For now you'll have to wait for the bot to be added to a channel to get the token.


AvengingPrime-Feature/Content_Viewer

This closes #29, and other sub-issues that have been merged to the current branch- #31 #32 #33 #37.

This is what the current components look like when given relevant prop values-

Also, I have added a fonts folder in the styles folder. The fonts folder contains the truetype font files, and a styling file that declares those font faces. I have also added an images folder in the public folder, that currently contains the share image. Other images we use throughout the site (not the ones in articles) will probably go in the same location. So, if the locations of these folders or their organisation needs to change, let me know


vulnsrc: Refactor debian and alpine sources

Add fsutil to handle listing directory function
Add FindKeyValue wrapper in database module
Refactor debian and alpine sources to use them

Updated according to comments


Add Koto

I understand.
We will change these constants at next network upgrade.

:+1:

But, we would like to use a trezor with Electrum for Koto. Can we use it?

Yes


Depends on WFMP 4.1.6.Final

Github is hanging to enable the Merge Pull request button., Closing this one, will re-open one.


Attempt to fix flaky AccessSubmissionDashboard tests

Bump 'history' package to v4, v5 is not compatible with React Router v5
Remove test skips
waitFor assertions to check the props passed to a mock component

Locally, I ran multiple iterations of the AccessSubmissionDashboard tests. I experienced irregular test failures with 'history' v5 that went away when I changed the version of history to v4. The documentation for the history package notes that v4 is compatible with React Router v5. If/when we upgrade to react-router v6, we should bump history back to v5.
Using waitFor for the failed assertions should also help in the comparatively under-resourced CI environment.

Note that I ran the test job on the latest commit 5 times and got 5 consecutive successful runs: https://github.com/Sage-Bionetworks/synapse-web-monorepo/actions/runs/7432473054/job/20265252842?pr=641


Hotkeys

Here is the change with just the hotkeys added

There are 3 commits, but the 2nd one is me getting rid of my changes and pulling the current master


Fix Pruning callback and add a few features

What does this PR do?
Many test fixes
Many refactors
TODO:

[x] Make lottery a Callable + tests
[x] Support iterative pruning + tests
[x] Save pruned model on checkpoint
[x] Check docs look good
[x] Sparsity history and tracking
[x] Allow resampling
[ ] Full model statistics, more verbose level
[ ] Support loading non-permanent pruned checkpoint
[ ] Second chance? edit: left for a future PR (maybe?)
[ ] CHANGELOG edit: No need since pruning hasn't been released yet

Before submitting

[x] Was this discussed/approved via a GitHub issue?: Discussed offline
[x] Did you read the contributor guideline, Pull Request section?
[x] Did you make sure your PR does only one thing, instead of bundling different changes together?
[x] Did you make sure to update the documentation with your changes? (if necessary)
[x] Did you write any new necessary tests? (not for typos and docs)
[x] Did you verify new and existing tests pass locally with your changes?
[x] Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

[x] Is this pull request ready for review? (if not, please submit in draft mode)
[x] Check that all items from Before submitting are resolved
[x] Make sure the title is self-explanatory and the description concisely explains the PR
[x] Add labels and milestones (and optionally projects) to the PR so it can be classified
[x] Check that target branch and milestone match!

Did you have fun?
Make sure you had fun coding 🙃

Hello @carmocca! Thanks for opening this PR.

In the file pytorch_lightning/callbacks/pruning.py:

Line 316:13: W503 line break before binary operator
Line 317:13: W503 line break before binary operator
Line 318:13: W503 line break before binary operator

Do see the Hitchhiker's guide to code style

it would be much easier to have each of the 5 features as a separate PR

it would be much easier to have each of the 5 features as a separate PR

Consider this as a Pruning make-over more than adding individual features. The TODO above was just for me to track pending things

it would be much easier to have each of the 5 features as a separate PR

Consider this as a Pruning make-over more than adding individual features. The TODO above was just for me to track pending things

yes but it now because there is some code already, so remove all in one PR and then do this, now the reading with seeing past state is hard...

Line 394:13: W503 line break before binary operator
Line 395:13: W503 line break before binary operator

This is a false positive. This rule should be ignored:
https://github.com/PyTorchLightning/pytorch-lightning/blob/388eeea52d3abe5e8c08197ed06dc9ca9eeefe1c/setup.cfg#L83


[ Bug ] Level Up leaderboard interfere ingame graphic

I don't know what to do. This leaderboard i cannot hide - it is still in the picture even if i press hide it. See the picture below:

I cannot reproduce this issue. due you use any customer them or altered the CSS or HTML in any way?

I will try redownload IT and let you know.
On Tue, 4 Jul 2023, 19:59 Maximilian Bala, @.***> wrote:

I cannot reproduce this issue. due you use any customer them or altered
the CSS or HTML in any way?
—
Reply to this email directly, view it on GitHub
https://github.com/RCVolus/league-prod-toolkit/issues/423#issuecomment-1620587861,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AY4SY62H4RTCUN27WIFEQLTXORKZZANCNFSM6AAAAAAZ4TSNAE
.
You are receiving this because you authored the thread.Message ID:
@.***>

Today i will try new version and let you know if issue is still present or not.

Issue is gone.


how to add new locale?

how can i add new locale if not exits

create a function within src/components/datepicker/locale/index.ts  && export it

Could you add an example to do this in the docs? This would help a lot of people, I'd suggest :-)

updated readme with the steps to add new Locale, i hope this will be helpful.

Hi. Thank you. As stated here (https://github.com/shubhadip/vuejs3-datepicker/pull/32), I think most people (including me) do not know how to actually use the new locale :-)

Also adding all the locales like this will prevent locales from being treeshaked, so all locales will always be loaded on one's page


[bug] cache files have absolute paths

Inside the cache files, there is an origin and path property that has an absolute path in it. I think that this path should really be relative to baseDir.

Well that is on purpose, why do you think we should have relative path there? When the path differs, it will trigger cache reload for that entity (it should at least :) ).
The idea was to force regenerating cache when changing baseDir or moving the project to different folder (e.g. deployment).

With that logic, if you move it to another directory, it invalidates the cache. It doesn't matter what the path is then. Deployment would break the cache anyway, so why even deploy the cache?
The data in the cache is only relative to baseDir to begin with. It doesn't need an absolute path to check for invalidation. If you change baseDir, why would that invalidate the cache?

We're back to my favorite quote: https://twitter.com/secretGeek/status/7269997868

Deployment would break the cache anyway, so why even deploy the cache?

One should not do that obviously. This is more of a fail safe thing so it still works even if one commits/deploys the cache.

If you change baseDir, why would that invalidate the cache?

Because you cannot know that possibly found entity in the new path is the same one as the one in old path.
Again, more of a fail safe that should not be triggered, but this could happen:

I have 2 entities with same name on two different locations, lets say A and B
A is in location based on baseDir
user changes baseDir so B is now in location based on baseDir

Cache won't be regenerated, but A and B might not be compatible.

This is more of a fail safe thing so it still works even if one commits/deploys the cache.

What would break if you deploy the cache and the paths match up? Seems like at some level, the cache might even be worth checking in just for sanity to get diffs on.

example....

I think that is a stretch. If there was a bug for that, then I'd consider fixing it in another way (like a simple hash on the file contents). I don't think that clogging the cache files with absolute paths is the solution for it.

Also realize that there are serverless environments that don't allow temp directories or don't benefit from it. I was even thinking of creating a firebase metadata backing for the cache....

I think that is a stretch. If there was a bug for that, then I'd consider fixing it in another way (like a simple hash on the file contents). I don't think that clogging the cache files with absolute paths is the solution for it.

Right, that sounds like a way to go, md5 hash of file contents as origin value and we can use relative paths in metadata.

Also realize that there are serverless environments that don't allow temp directories or don't benefit from it. I was even thinking of creating a firebase metadata backing for the cache....

That is the reason why you can 1. disable cache and 2. implement your own cache adapter.

Implemented in 3.0.0-alpha.15, as always, feel free to reopen.


Encouragement: Awesome package!

This is a fantastic, high quality Material Design implementation. Thank you for your hard work :)

Thank you!


Consider how hints interact with motors that have a fixed relation

Lutz, suggesting how the BEC plot could better display his data:

I imagine a simple line plot, like detector intensity (y-axis) vs. dcm_b (x-axis), with 2 additional x-axes (ivu_gap, energy). dcm_b, gap and energy all have a fixed relation to each other.

My first thought is that if ivu_gap, energy, and dcm_b have a "fix relation" than they should be part of one Device. And that Device should express in its hints that there is one axis here.
This is a use case that is likely to reappear, so I wanted to flag it for future consideration. This does not seem urgent.

Specifically, this is implement in the BestEffortCallback like so....
If a dimension has multiple fields that represent one shared degree of freedom (like diffractometer position and energy), we currently drop all fields except the first one:
self._dim_fields = [fields[0] for fields, stream_name in dimensions]

We could use shared axes feature of matplotlib to represent this better.


[Feature][Registration Center] Suggested that the Kubernetes application mode supports s3 or http

Search before asking

[X] I had searched in the issues and found no similar feature requirement.

Description
Since flink 1.19,k8s application can be submitted by s3 or http.https://nightlies.apache.org/flink/flink-docs-release-1.19/docs/deployment/resource-providers/native_kubernetes/
Suggested that the Kubernetes application mode supports s3 or http in Dinky.
Use case
Dinky in Docker is unable to upload jar file.Jar file are usually uploaded to s3 or other file service.
Related issues

Are you willing to submit a PR?

[ ] Yes I am willing to submit a PR!

Code of Conduct

[X] I agree to follow this project's Code of Conduct

你要来实现吗

好我试试

可以使用 init container , 或者自己打一个包含 app.jar 的dinky 镜像.
如果你对这个有兴趣可以来试着贡献下,但是需要等到 dinky-release-1.0.2 发布之后,或者基于 dev 分支,因为在上述版本中才添加了 Flink1.19 的支持,同时你需要考虑不同的版本如何兼容

我今天看了下源码并做了一些测试，发现使用s3或http提交jar包到kubernetes application和dinky没什么关系，flink支持就行，也许可以关闭这个issue了


Fix solid-ssr on windows

As discussed on discord, the static adapter for solid-start breaks because writeToDisk.mjs fails on windows (silently, because the asynchronous version of the function is called.
I really have no idea what I'm doing here, I just made changes that worked for me. Please review and determine the best solution!

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 89.587%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
1215

Relevant Lines:
1294

💛  - Coveralls


Unrecognised member "token"

After upgrading the fcm plugin to 2.0.0, Xcode fails to recognise the member token in the Plugin.swift file.
Value of type 'Messaging' has no member 'token' on line 62.

I have followed the iOS setup procedure as stipulated in the docs. Any assistance would be appreciated.

check your Podfile.lock, we have upgrade firebase so it must show references of  FirebaseMessaging 8
try to

cd ios/App
pod cache clean
pod install

As a closing comment, I had to run pod update. Thanks for the assistance.


Generate a self signed cert on ds creation/update

What does this PR (Pull Request) do?

[x] This PR fixes #5893

This PR updates TO so that when a new HTTPS delivery service is created or an HTTP delivery service is updated to HTTPS, a self signed certificate is created as a placeholder.
Which Traffic Control components are affected by this PR?

Traffic Ops

What is the best way to verify this PR?

Create a new HTTPS DS and verify that a self signed certificate is generated and stored
Create an HTTP DS and verify that no cert is generated
Update that HTTP DS to use HTTPS and verify that a self signed cert is generated and stored
Update an existing HTTPS DS that already has a cert and verify that the previous cert remains unchanged
Verify that all other DS creation / update functionality is unchanged
If this is a bug fix, what versions of Traffic Control are affected?

The following criteria are ALL met by this PR

This test does not include docs or tests since it is a minor change that will not impact these

[x] This PR includes tests OR I have explained why tests are unnecessary
[x] This PR includes documentation OR I have explained why documentation is unnecessary
[x] This PR includes an update to CHANGELOG.md OR such an update is not necessary
[x] This PR includes any and all required license headers
[x] This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

Would there be a way to configure these so the CSR is valid to use after being auto-generated? That would mean new configuration.
			BusinessUnit:    util.StrPtr("Placeholder"),
			City:            util.StrPtr("Placeholder"),
			Organization:    util.StrPtr("Placeholder"),
			Country:         util.StrPtr("Placeholder"),
			State:           util.StrPtr("Placeholder"),

I like that idea. Just make the new config optional and use placeholder by default 👍

Closes #2429