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

Mapper: Comments on fields of bean types #18

Closed
ljacqu opened this issue Oct 16, 2016 · 8 comments
Closed

Mapper: Comments on fields of bean types #18

ljacqu opened this issue Oct 16, 2016 · 8 comments
Labels
beanmapper Issues that relate to the bean mapper
Milestone

Comments

@ljacqu
Copy link
Member

ljacqu commented Oct 16, 2016

Should we support @Comment on bean fields? This might be difficult since we don't always have a direct path (maps with unknown keys) and the comment would be present for each entry.

@sgdc3
Copy link
Member

sgdc3 commented Nov 19, 2020

Yeah this should be supported

@ljacqu
Copy link
Member Author

ljacqu commented Nov 22, 2020

If you have any good use cases, please post them. Definitely one of the bigger things missing, I'd just like to have some good examples to think about how this feature would look in detail

@sgdc3
Copy link
Member

sgdc3 commented Nov 30, 2020

I think @gamerover98 might have some specific examples

@gamerover98
Copy link
Contributor

Hello, this is a piece of my configuration where I've used the bean implementation:

TeamProperty class:

 public static class TeamProperty implements ParameterizedType {

	private static final Type[] TYPE_ARGUMENTS = {
		String.class,  //id
		String.class,  //name
		String.class,  //chat color
		DyeColor.class //dye color
	};

	private String    key;
	private String    name;
	private ChatColor chatColor;
	private DyeColor  dyeColor;

	@SuppressWarnings("unused")
	public TeamProperty() {
		// nothing to do
	}

	public TeamProperty(@NotNull String key, @NotNull String name,
			    @NotNull ChatColor chatColor, @NotNull DyeColor dyeColor) {

		this.key       = key;
		this.name      = name;
		this.chatColor = chatColor;
		this.dyeColor  = dyeColor;

	}

	@Override
	public Type[] getActualTypeArguments() {
		return TYPE_ARGUMENTS;
	}

	@Override
	public Type getRawType() {
		return TeamProperty.class;
	}

	@Override
	public Type getOwnerType() {
		return null;
	}

}

BeanPropertyType implementation:

public static class TeamPropertyType extends BeanPropertyType<TeamProperty> {

        private static final Type TYPE = new TeamProperty();

	public TeamPropertyType() {
		super(new TypeInformation(TYPE), DefaultMapper.getInstance());
	}

}

In addition, I've implemented it into a SettingsManager class with a property array:

public class TeamsHolder implements SettingsHolder {

    private static final Type TYPE = new TeamProperty();

    private static final String TEAMS_VALUE = "teams";

    public static final Property<TeamProperty[]> TEAMS;

    static {

        TeamProperty[] defaultTeamPropertyArray = new TeamProperty[] {
                new TeamProperty("RED",  "red",  ChatColor.RED,       DyeColor.RED),
                new TeamProperty("BLUE", "blue", ChatColor.DARK_BLUE, DyeColor.BLUE)
        };

        TEAMS = new PropertyBuilder.ArrayPropertyBuilder<>(new TeamPropertyType(), TeamProperty[] :: new)
                .path(TEAMS_VALUE)
                .defaultValue(defaultTeamPropertyArray)
                .build();

    }

}

This is the result:

teams: 
- key: RED
  name: red
  chatColor: RED
  dyeColor: RED
- key: BLUE
  name: blue
  chatColor: DARK_BLUE
  dyeColor: BLUE

I want to add comments like:

teams: 
  #The team key
- key: RED
  #The team name that...
  name: red
  #Chat color messages...
  chatColor: RED
  #The armor leather color
  dyeColor: RED

Thank you for asking

@gamerover98
Copy link
Contributor

Heard anything? 😌

@ljacqu ljacqu added the beanmapper Issues that relate to the bean mapper label Oct 17, 2021
@ljacqu
Copy link
Member Author

ljacqu commented Oct 17, 2021

This is really good input, thanks. The reason I tend to forget about this issue is that I'm still unclear on how we'd handle multiple entries in the array.
Need a way to define whether comments should be put on every entry or only on the first. Maybe an optional config on @Comment, except I don't like that the option would then also be available on properties (where it makes no sense). An additional annotation isn't much better since it's hard to name it and the problem, at its core, remains (someone could get confused and use the other annotation on property fields).
Tentatively scheduling this for the next major release since IMHO it is still something that's missing from a complete feature set. 😞 I'm a little paralyzed by indecision on the design.

@ljacqu ljacqu added this to the 1.4.0 Release milestone Oct 17, 2021
@ljacqu ljacqu self-assigned this Jun 30, 2023
@ljacqu
Copy link
Member Author

ljacqu commented Jul 2, 2023

I've been thinking about this a lot, and I think it would be interesting to be able to define comments based on the property values. This would be helpful for #132, but also here. Before exporting, some method could be called that collects @Comment annotations, incl. bean properties. Probably add an option to @Comment to define whether it should appear once or many times. I don't see a more straightforward way.
I will now see how to best have such a method but also make sure that bean properties just automatically get comments registered if they have an @Comment annotation on any fields. Maybe I'm barking up the wrong tree with the idea of defining comments just before an export happens, but I think it would open the door for other things (like the linked issue).

@ljacqu ljacqu changed the title Mapper: Analyze comments on fields Mapper: Comments on fields of bean types Jul 3, 2023
ljacqu added a commit that referenced this issue Jul 4, 2023
- Missing new tests & adaptions of existing tests
- Comments on map values currently breaks the export
ljacqu added a commit that referenced this issue Jul 4, 2023
- Draft version of exporting properties as SnakeYAML nodes (allows to specify comments)
- Current state still has many open points
ljacqu added a commit that referenced this issue Jul 4, 2023
- Fix issues with new lines
- Remove unused code
ljacqu added a commit that referenced this issue Jul 5, 2023
- Restructure logic, revise Javadoc
- Still missing a lot of tests
ljacqu added a commit that referenced this issue Aug 3, 2023
- Introduce SnakeYamlNodeBuilder to create nodes for every export value
- Properties can return ValueWithComments to specify comments on the fly
- Missing tests and ability to not repeat a comment in a bean
@ljacqu
Copy link
Member Author

ljacqu commented Aug 3, 2023

Breaking changes

You should not be affected by any breaking changes if you are using ConfigMe "normally" and not overriding/implementing classes like YamlFileResource. Otherwise, please read on. Note that further breaking changes may be introduced with ConfigMe 2.0, so if you don't need any of the new features of ConfigMe 1.4, consider updating from 1.3 to 2.0 directly once it is out.

Breaking changes:

  • YamlFileResourceOptions#getIndentation (returning String) has been removed
  • YamlFileResource now represents all values as SnakeYAML nodes instead of piecing YAML together bit by bit. As such, many protected methods have been changed or removed, such as String toYaml(Object) or void writeComments(Writer, int, PathElement).
  • If you override YamlFileResource#createNewYaml, note that options.setProcessComments(true) and
    options.setIndent(indentationSize) have been added to the dumper options. You probably want to add that to your extensions, too!
  • PropertyPathTraverser uses different logic to produce objects of PathElement (nested class). PathElement itself has had some changes in its fields and methods.
  • The interface BeanPropertyDescription has a new method List<String> getComments()
  • Because the whole model is now exported as one by SnakeYAML, there are slight differences to the YAML that is written; some trailing whitespaces are now eliminated, whereas some comments might be slightly indented differently.
  • Inner workings of MapperImpl have changed (this affects some protected methods). Some have a new argument of type Set<UUID>.

If these changes cause issues for you (because you are overriding YamlFileResource or otherwise relying on these classes), please open a new issue to get assistance.

ljacqu added a commit that referenced this issue Aug 9, 2023
ljacqu added a commit that referenced this issue Aug 10, 2023
@ljacqu ljacqu closed this as completed Aug 10, 2023
@ljacqu ljacqu removed their assignment Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beanmapper Issues that relate to the bean mapper
Development

No branches or pull requests

3 participants