Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow subobject nesting (subsubobjects) #3702

Closed
Larivact opened this issue Feb 10, 2019 · 8 comments
Closed

Allow subobject nesting (subsubobjects) #3702

Larivact opened this issue Feb 10, 2019 · 8 comments
Labels
enhancement Alters an existing functionality or behaviour good first issue Good for newcomers

Comments

@Larivact
Copy link

Larivact commented Feb 10, 2019

Setup and configuration

  • SMW version: 3.0.0

Issue

The #subobject parser function apparently cannot be nested, which can be desirable.

Syntax suggestion:

{{#subobject:
|Has property 1=value
|Has property 2=values
|{{#subobject:
 |Has property 1=value
 |Has property 2=values
}}
}}
@Larivact Larivact changed the title Allow for nested subobjects (subsubobjects) Allow subobject nesting (subsubobjects) Feb 10, 2019
@mwjames
Copy link
Contributor

mwjames commented Feb 10, 2019

{{#subobject:
|Has property 1=value
|Has property 2=values
|{{#subobject:
 |Has property 1=value
 |Has property 2=values
}}
}}

This won't work as each entity (meaning a subobject) needs a context of the embedding instance but you run into the same problems as described in (#3654) about how parser functions work in MW. The second #subobject is unable to determine that its context object is the first #subobject and not the embedding subject (== wiki page).

There are other issues but (without having delved into it) I would guess above is a general show stopper for the proposed syntax .

@Larivact
Copy link
Author

I guess a good alternative would be a return identifier = yes parameter for #subobject, which would allow for:

{{#subobject:
|Has property 1=value
|Has property 2=values
|Has subthing={{#subobject:
 |Has property 1=value
 |Has property 2=values
 |return identifier=yes
}}
}}

@Larivact Larivact changed the title Allow subobject nesting (subsubobjects) #subobject should be able to return identifier when asked to Feb 10, 2019
@mwjames
Copy link
Contributor

mwjames commented Feb 10, 2019

I guess a good alternative would be a return identifier = yes parameter for

It doesn't work like this, the second #subobject needs the reference of the first. In your example the first gets the reference of the second while semantics for the subtree are subject -> #subobject (first) -> #subobject (second), so the second has to "know" that it belongs to the first (which it cannot as outlined above).

@Larivact
Copy link
Author

Larivact commented Feb 10, 2019

Note that I wrote Has subthing not Has subobject. While these two subobjects would be siblings in the subtree, return identifier = yes would still facilitate the construction of a hierarchy between subobjects outside of the subtree.

@mwjames
Copy link
Contributor

mwjames commented Feb 10, 2019

Besides the technical aspect, what is the use case here? What sort of data would require such construct?

@Larivact
Copy link
Author

Articles that mention books with multiple editions.

@mwjames
Copy link
Contributor

mwjames commented Feb 10, 2019

The following is a quick hack (untested == contains caveats yet to be found, no guarantees) but it should provide an idea of what is required to make the semi tree structure available (for example the change fails for SG [0]).

For those who want (it ain't me) to implement this:

  • Test the code path (I mean it!! and improve it, it is just a rough sketch)
  • Make field tests with existing store implementations
  • Write appropriate integration tests
  • Test with other extensions (see [0])
  • Limit the hierarchy depth, and so on ...

Whether this is a functionality we want to support or not (maintenance cost, hidden technical debt, errors that may arise due to hierarchy dependencies etc.) is a different discussion.

Changes

@@ -46,10 +46,12 @@ class SubobjectParserFunction {
 	 * Fixed identifier that describes a property that can auto-linked the
 	 * embeddedding subject
 	 */
 	const PARAM_LINKWITH = '@linkWith';
 
+	const PARAM_EMBEDDED = '@embedded';
+
 	/**
 	 * @var ParserData
 	 */
 	protected $parserData;
 
@@ -81,10 +83,12 @@ class SubobjectParserFunction {
 	/**
 	 * @var boolean
 	 */
 	private $isComparableContent = false;
 
+	private $embedded = false;
+
 	/**
 	 * @since 1.9
 	 *
 	 * @param ParserData $parserData
 	 * @param Subobject $subobject
@@ -167,11 +171,11 @@ class SubobjectParserFunction {
 		// An empty output in MW forces an extra <br> element.
 		//if ( $html == '' ) {
 		//	$html = '<p></p>';
 		//}
 
-		return $html;
+		return $html . $this->embedded;
 	}
 
 	protected function addDataValuesToSubobject( ParserParameterProcessor $parserParameterProcessor ) {
 
 		// Named subobjects containing a "." in the first five characters are
@@ -190,10 +194,11 @@ class SubobjectParserFunction {
 
 		$this->subobject->setEmptyContainerForId(
 			$id
 		);
 
+		$this->embedded = $this->embedded ? $id : '';
 		$subject = $this->subobject->getSubject();
 
 		foreach ( $parameters as $property => $values ) {
 
 			if ( $property === self::PARAM_SORTKEY ) {
@@ -202,10 +207,19 @@ class SubobjectParserFunction {
 
 			if ( $property === self::PARAM_CATEGORY ) {
 				$property = DIProperty::TYPE_CATEGORY;
 			}
 
+			if ( $property === 'Has subobject' ) {
+				$subSemanticData = $this->parserData->getSemanticData()->findSubSemanticData( end( $values ) );
+				$this->subobject->getSemanticData()->addPropertyObjectValue(
+					new DIProperty( '_SOBJ' ),
+					new \SMWDIContainer( $subSemanticData )
+				);
+				continue;
+			}
+
 			foreach ( $values as $value ) {
 
 				$dataValue = DataValueFactory::getInstance()->newDataValueByText(
 						$property,
 						$value,
@@ -251,10 +265,15 @@ class SubobjectParserFunction {
 		return [ $parameters, $id ];
 	}
 
 	private function preprocess( ParserParameterProcessor $parserParameterProcessor, $useFirst ) {
 
+		if ( $parserParameterProcessor->hasParameter( self::PARAM_EMBEDDED ) ) {
+			$this->embedded = true;
+			$parserParameterProcessor->removeParameterByKey( self::PARAM_EMBEDDED );
+		}

Usage

{{#subobject:
 |Has text=test
 |Has subobject={{#subobject:
  |Has page=123
  |@embedded=true
  |Has subobject={{#subobject:
   |Has page=123,1234|+sep=,
   |@embedded=true
  }}
 }}
}}
{{#ask: [[Has text::test]]
 |?Has text
 |?Has subobject
 |?Has subobject.Has page
 |?Has subobject.Has subobject.Has page
}}

{{#ask: [[Has page::123]]
 |?Has page
 |?Has subobject
 |?-Has subobject.Has text
 |?-Has subobject.Has subobject.Has text
}}

image

Notes

[0] as it returns "[39e98b4cffe23e9ab6c97a72] /mw-master-pg2/index.php?title=Subsubobject&action=submit TypeError from line 81 of ...\extensions\SemanticGlossary\src\Cache\CacheInvalidator.php: Argument 2 passed to SG\Cache\CacheInvalidator::invalidateCacheOnStoreUpdate() must be an instance of SMW\SemanticData, null given, called in ...\extensions\SemanticGlossary\src\Cache\CacheInvalidator.php on line 135")

@Larivact Larivact changed the title #subobject should be able to return identifier when asked to Allow subobject nesting (subsubobjects) Feb 10, 2019
@mwjames mwjames mentioned this issue Apr 9, 2019
@kghbln kghbln added enhancement Alters an existing functionality or behaviour good first issue Good for newcomers and removed question labels May 4, 2019
@kghbln
Copy link
Member

kghbln commented Aug 21, 2019

We need a developer for implementation. Thus I added this enhancement to the "Enhancements and features" project as "open". Closing here.

@kghbln kghbln closed this as completed Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Alters an existing functionality or behaviour good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants