morbac / xmltools

XML Tools plugin for Notepad++
GNU General Public License v3.0
260 stars 57 forks source link

Always show XPath Index when enabled, disable attributes #150

Closed JulianMa closed 2 years ago

JulianMa commented 2 years ago

Fix, discussed in https://github.com/morbac/xmltools/pull/149 I also disabled attributes when numeric predicates are enabled, as its not possible to display the attribute and index in one selector.

morbac commented 2 years ago

There is some issues with this PR:

1 - It seems indexed xpath cannot be disabled : I set False in options, but xpath still contains index infos

2 - The option is active for "Current XML Path with predicates" only. It is ignored for "Current XML Path" function.

3 - The attributes remove is not mandatory. Actually XPaths can have position and attributes simultaneously. Example:

<a>
  <b c="x">b_1</b>
  <b c="y">b_2</b>
</a>

If cursor is on y, path should be /a/b[2]/@c

4 - Position attribute is sometimes used when not necessary. Example:

<a>
  <b>
    <c>1</c>
    <d>2</d>
  </b>
</a>

If I put cursor on value 2, XPath gives /a/b[1]/d[1], but position is not required here, path /a/b/d is precise enough.

As a conclusion, I think indexed xpath construction algorithm should be improved. During construction, each xpath should be checked: if it returns more than one node, index is required, otherwise not. For instance:

<a>
  <b>
    <c d="x1"/>
    <c d="y2"/>
  </b>
  <b>
    <c d="x3"/>
    <c d="y4"/>
  </b>
</a>

Cursor is in node /a/b[2]/c[2]/@d (value y4) First we identify node a. Since it is root, it is unique. Next we indentify b and obtain /a/b. Since this path returns two nodes, we must add index: /a/b[2] Next we have node c. The path /a/b[2]/c returns two nodes too, index is required: /a/b[2]/c[2] Finally we add attribute @d and obtain path /a/b[2]/c[2]/@d

morbac commented 2 years ago

Just one more thing : the path displayed in statusbar was not intended to be a real xpath. For real path, I recommand using "Current XML Path" as I did in previous message. I though you were adding index information to "Current XML Path" function since this xpath has to be precise enough to identify a unique node.

The goal behind statusbar path feature was to dump a path that can help user to know in which node cursor is placed. In my job, I frequently work on very repetitive xml files, with lot's of similar elements indentified with @id attribute. Here's an example:

image

The path shown in statusbar is not intended to be a precise xpath. It's just a tip to help me know in which application, Model, Page and CodeLine my cursor is placed. Index is not useful in such situation since it doesn't gives useful information in my opinion.

If you don't mind, I think I'll remove the "indexed xpath" from statusbar path and add it in "Current XML Path" (with or without predicates) function only.